Comment by WhyNotHugo
11 hours ago
I really don't get the point of stacked PRs.
Just using git, you'd send a set of patches, which can be reviewed, tested and applied individually.
The PR workflow makes a patch series an undivisible set of changes, which must be reviewed, tested and applied in unison.
And stacked PRs tries to work around this issue, but the issue is how PRs are implemented in the first place.
What you really want is the ability to review individual commits/patches again, rather than work on entire bundles at once. Stacked PRs seems like a second layer of abstraction to work around issues with the first layer of abstractions.
The teams that I have worked with still apply the philosophy you’re describing, but they consider PRs to be the “commit”, i.e. the smallest thing that is sane to apoly individually.
Then the commits in the PR are not held to the standard of being acceptable to apply, and they are squashed together when the PR is merged.
This allows for a work flow in which up until the PR is merged the “history of developing the PR” is preserved but once it is merged, the entire PR is applied as one change to the main branch.
This workflow combined with stacked PRs allows developers to think in terms of the “smallest reviewable and applicable change” without needing to ensure that during development their intermediate states are safe to apply to main.
Exactly! A stack of PRs is really the same beast as a branch of commits.
The traditional tools (mailing-lists, git branches, Phabricator) represented each change as a difference between an old version of the code and the proposed new version. I believe Phabricator literally stored the diff. They were called “diffs” and you could make a new one by copying and pasting into a <textarea> before pressing save*.
The new fangled stuff (GitHub and its clones) recorded your change as being between branches A and B, showed you the difference on the fly, and let you modify branch B. After fifteen years of this we are now seeing the option for branch A to be something other than main, or at least for this to be a well supported workflow.
In traditional git land, having your change as a first class object — an email or printout or ph/D1234 with the patch included — was the default workflow!
*Or some other verb meaning save.
It’s useful for large PRs in large repos with many contributors. It reduces the burden for reviewers.
Still not sure this is the right solution. My problem is if one of your first stages gets rejected in review or requires significant changes, it invalidates so much work that comes after it. I've always when possible preferred to get small stuff merged in to production as it happens rather than build an entire feature and put it up for review.
this works much better in Phabricator because commits to diffs are a 1:1 relationship, diffs are updated by amending the commit, etc., the Github implementation does seem a bit like gluing on an additional feature.
Right, a PR is "just" a set of commits (all must be in the same branch) that are intended to land atomically.
Stacked PRs are not breaking up a set of commits into divisible units. Like you said, you can already do that yourself. They let you continue to work off of a PR as your new base. This lets you continue to iterate asynchronously to a review of the earlier PRs, and build on top of them.
You often, very often, need to stage your work into reviewer-consumable units. Those units are the stack.