How to Review Stacked Pull Requests When the Base Moves

The last stack I worked through was on GitHub, four or five layers deep. While the upper layers were still in review, main moved on and the stack was rebased, and a lower layer merged. One of the upper layers needed another review because of it, and I nearly approved it without noticing that anything below it had changed.

Its diff gave me no reason to. In a stack, a layer's own diff can stay the same while the code underneath it changes, and most of what makes stacks tricky to review follows from that.

How a stack is put together

Each pull request in a stack targets the branch of the one below it, and only the bottom one targets main. GitHub has native stacks in public preview, where each layer shows only the diff between its branch and the branch under it, and branch protection, CODEOWNERS approvals included, is enforced on every layer. GitLab gets the same shape from merge requests chained by target branch, and Gerrit from a relation chain built on commit ancestry.

You get small reviews in exchange for more state. A single pull request has one owner of the next move. In a stack it can be a different person on every layer: layer 1 waiting on you, layer 2 on its author, layer 4 on CI, and layer 3 approved on top of a version of layer 2 that has since been rewritten.

Because every diff is relative to the layer below, I review from the bottom up. Comments on layer 1 can make anything I said about layer 3 obsolete. I do read the top layer's description first, since a layer that adds an interface with one caller makes more sense once you know the second caller arrives in layer 4. And when a lower layer gets comments that will change its shape, I stop there until the next push, or I end up reading parts of layer 3 twice.

When the ground moves under a layer

The simplest version is a lower layer getting review fixes. The author of layer 2 addresses your comments. Layer 3 sits on the old layer 2 until someone rebases it, and afterwards its diff can look identical to the one you read. What you are really checking is whether layer 3 relied on anything that changed in layer 2. A renamed function breaks the build, so if CI runs on the rebased layer you'd hear about that one. A changed default or a new error path doesn't.

Main moving is broader. The whole chain gets replayed on a newer main, and if conflicts were resolved on the way, those resolutions now sit in layers that may already be approved. Asking the author which layers had conflicts is the quick way to find them, and the slower way is below. On a single pull request this is the ordinary re-review after a rebase. In a stack it can land on three layers at once.

Then there is the bottom layer merging, which is how stacks are supposed to land. When a layer of a GitHub native stack merges, the next one is rebased automatically to target main. GitLab retargets up to four open merge requests when the branch they point at is merged, but only as part of the merge. Deleting the source branch afterwards retargets nothing. On GitHub, if that rebase went cleanly, the next layer's diff is unchanged. If it didn't, there are lines in it you've never read.

Finding out what actually moved

The diff of the layer in front of you won't show any of this, so look at the layers themselves.

For the lower layer, it depends how it was updated. Plain new commits show up in its commit list like any other push. A force-push shows up in the timeline, and on GitHub clicking force-pushed opens a comparison between the old head and the new one. If layer 2 was only fixed, that comparison is the change layer 3 now sits on. If layer 2 was also rebased onto main, main's changes are mixed into it, and running range-diff on layer 2 (below) separates them out, since main's changes sit in the bases, outside the commits it compares. GitLab keeps a version of the merge request for every push, and the Compare dropdown on the Changes tab diffs any two of them.

For the upper layer after a rebase, use git range-diff. Point it at the layer's commits before and after the rebase:

git range-diff <old-base>..<old-head> <new-base>..<new-head>

Each commit gets a line. = means the rebase carried it over unchanged and ! means it now differs. A < or > marks a commit that exists on only one side. The ! lines are the ones to read, and conflict resolutions end up there. For layer 3, the old base is layer 2's head before the change and the new base is layer 2's head now. Both old heads are in the force-push comparisons on each layer, or in your local reflog if you fetched them.

Range-diff tells you what the rebase did to layer 3's own commits. It says nothing about what changed underneath. If layer 3 comes through with every line marked =, that is the case I opened with, and the check left is layer 2's comparison. If layer 2 has already merged, read what it merged as instead.

Gerrit does part of this for you. Its related changes list marks an ancestor as not current once a newer patch set replaces the one you built on, and the documentation spells out that the change you're viewing now needs a rebase.

Your approval is the other thing to check. GitHub has an optional branch protection setting that dismisses an approval when new commits change the diff, and new changes arriving through the merge base count too. With it on, a rebased layer whose diff picked up changes needs approving again. The docs don't say how it treats a stacked layer whose own diff came through untouched while the layer under it changed, so I wouldn't lean on it for that case. With it off, the approval stays and GitHub asks nobody to look again, so it's worth reading what changed in the layer below, and any ! commits, before you let it stand.

Someone in an r/programming thread about GitHub's stacked pull requests asked for something smaller than any of this: just the diff since the last commit they'd looked at, instead of everything between source and target. In a stack, one commit isn't enough. You want the head you reviewed on your layer and the head of the layer under it at the time.

When I wouldn't stack

If you can't approve layer 1 without reading layer 3, the stack is one change split across three tabs, and a single pull request with clean, separate commits reads better. I'd also think twice on a team where pull requests already sit for days between reviews, because every layer waits in its own queue before anyone gets to the top.

Seeing where a stack stands

PR Flow reads stack membership from GitHub's native stacks, GitLab's chained merge requests and Gerrit's relation chains. Azure DevOps doesn't report stacks, so there it infers a chain from branches only when that's unambiguous, and labels it as inferred. Layers of one stack sit together in the queue, ordered from the foundation up, and opening any layer shows the map from trunk to top with each layer's review state. The stacked pull requests docs have the details.

It also warns you while a lower layer has changed and the layer you're reading hasn't been rebased yet: this layer may need a rebase before your review is current. It goes by timestamps, so it can't tell rework from reformatting, and it disappears as soon as the upper layer is pushed again, rebased or not.

After the rebase, it follows your provider. On GitHub, if the approval was dismissed, PR Flow stops counting it and treats the layer as one you haven't reviewed. If your approval stands, the rebase alone doesn't bring the layer back to your Needs you list. PR Flow won't show you what moved underneath or which hunks were conflict resolutions. The case I opened with, a rebased layer whose diff looks the same, is left to you and the layer below it.

I'm not sure what should flag that case. A clean rebase can be the dangerous one, because nothing in the upper layer changes. A warning on every rebased layer would catch it and would also fire on every harmless rebase, on top of a warning that already can't tell rework from reformatting. I suspect people would stop reading it.