spr only allows landing a commit when merging the PR as-is will produce the same result on master as cherry-picking the local commit. This ensures that changes made to the local commit after last PR update are neither silently landed nor discarded.
There is one edge case: if the local commit was previously based on another commit, that has now been landed, then Git may be able to merge the whole PR branch into master, understanding that there is a commit present in both master and the PR branch (including the base branch) since the latter branched off the former.
In that case spr can go ahead with the landing. If you check the PR on GitHub afterwards, it will show not only changes in the landed commit, but also in the base of the commit. Those had been landed on master already, but they are also shown now as part of this PR.
To get rid of this, we have to merge in the current master (which contains those other landed changes), so that those additional changes on base are no longer seen as coming from the PR branch (including the base branch).
spr used to add a commit titled "[spr] landed version" when landing any PR. This PR brings back that "landed version" commit, however: only when it is necessary to have it to correct the topology. Also, very importantly, it never introduces changes by the author into the PR. If there are last-minute changes, spr still refuses to land (you have to do spr diff first).
Thanks to @nickfil22 for reporting this issue.
Test Plan:
I created a local branch on master, added two commits with independent changes.
spr diff --all to create Pull Requests for both
using git rebase -i, I landed the first commit (I put x spr land in a line after the first commit)
after that the branch now contains only the second commit, and is based on the landed first commit in master
I call spr land to land the second commit.
Without the changes in this PR: the PR is squash-merged. When looking at the second PR in GitHub after squash-merging, it shows the changes from both commits.
With the changes in this PR: spr land adds a new commit to the PR and then squash-merges. When looking at the second PR in GitHub, it correctly only shows the changes in the second commit.
Another test is to do an spr diff on the second commit just before landing. This will show that the commit has been rebased and therefore it updates the PR.
With or without the changes in this PR: landing the second PR does not add a final rebase commit to the PR branch, and the contents of the PR after merging are displayed correctly.
spr only allows landing a commit when merging the PR as-is will produce the same result on master as cherry-picking the local commit. This ensures that changes made to the local commit after last PR update are neither silently landed nor discarded. There is one edge case: if the local commit was previously based on another commit, that has now been landed, then Git may be able to merge the whole PR branch into master, understanding that there is a commit present in both master and the PR branch (including the base branch) since the latter branched off the former. In that case spr can go ahead with the landing. If you check the PR on GitHub afterwards, it will show not only changes in the landed commit, but also in the base of the commit. Those had been landed on master already, but they are also shown now as part of this PR. To get rid of this, we have to merge in the current master (which contains those other landed changes), so that those additional changes on base are no longer seen as coming from the PR branch (including the base branch). spr used to add a commit titled "[spr] landed version" when landing any PR. This PR brings back that "landed version" commit, however: only when it is necessary to have it to correct the topology. Also, very importantly, it never introduces changes by the author into the PR. If there are last-minute changes, spr still refuses to land (you have to do
spr difffirst).Thanks to @nickfil22 for reporting this issue.
Test Plan:
spr diff --allto create Pull Requests for bothgit rebase -i, I landed the first commit (I putx spr landin a line after the first commit)spr landto land the second commit. Without the changes in this PR: the PR is squash-merged. When looking at the second PR in GitHub after squash-merging, it shows the changes from both commits. With the changes in this PR:spr landadds a new commit to the PR and then squash-merges. When looking at the second PR in GitHub, it correctly only shows the changes in the second commit.Another test is to do an
spr diffon the second commit just before landing. This will show that the commit has been rebased and therefore it updates the PR. With or without the changes in this PR: landing the second PR does not add a final rebase commit to the PR branch, and the contents of the PR after merging are displayed correctly.