Repository navigation
Replies: 5 comments 1 reply
|
Same issue after merging the first PR in the stack. |
|
Thank you so much for sharing this! forwarded this to the team so we can work on it 🙏🏼 |
|
And worse, trying to re-review a rebased PR using "Changes since your last review" will show a "We went looking everywhere, but couldn’t find those commits. Sometimes commits can disappear after a force-push." error page. |
|
This is still not working particularly well. When rebasing a stack that is fully approved by clicking the "rebase stack" in the UI, all approvals get dismissed and it mostly loses the entire benefit of having a stack. |
|
We narrowed down when this happens. Merging a lower PR dismisses an approval above it only when the upper PR changes lines within a few lines of the lower PR's change, even though the upper PR's diff is identical before and after the rebase. Approvals on upper PRs whose changes are farther away are kept, as the stacked PRs reference describes. Setup. A ruleset on Minimal repro (two PRs, one file):
Change step 3 to edit What we tested (each a stack of 2 or 3 PRs, with the bottom PR merged and the PRs above checked):
We saw the same dismissal on a real stack: a reviewer re-approved, the lower PR merged and the approval was dismissed, all with the upper PR's tree unchanged. It looks as though the stale check compares the diff hunks and their context against the old merge-base instead of comparing the diff against the new base. A stack where each PR builds on the one below edits nearby lines all the time, for example imports at the top of a file, so in practice most approvals above a merge are lost. We'd expect an approval to be kept whenever the rebased PR's diff against its new base is identical to the diff that was approved. |
Uh oh!
There was an error while loading. Please reload this page.
If I have an approved stack in a repo that has "Dismiss stale pull request approvals when new commits are pushed" enabled. When I rebased the entire stack, the first PR in the stack retained it's approval state meanwhile the downstream PRs dismissed their approval states and I have to re-request approvals.
All reactions