Comment by mytailorisrich 4 days ago
Stacked pull requests seem to add a layer of complexity to solve a problem that should and can be avoided in the first place.
Frequent, small changes are really a good practice.
Then we have things like trunk-based development and continuous integration.
Trunk-based development, by itself, is a fool's errand.
But combine it with TDD & pairing and it becomes a license to deliver robust features at warp speed.
I don’t follow. Regardless of where you merge, are you not developing features on a shared branch with others? Or do you just have a single long development branch and then merge once “you’re done” and hope that there’s no merge conflicts? But regardless, I’m missing how reviews are being done.
Stacked PRs allow me to post frequent, small changes without waiting for a review between each one.
Well, you don't need stacked PRs for that...
I think stacked PRs are a symptoms of the issues the underlying workflow (feature branches with blocking reviews) has.
“What you'll do next and in what way” is often an important tool to put the small changes into context.
Stacked pull requests can be an important tool to enable “frequent, small changes” IMO.
Sure, I can use a single pull request and a branch on top of that, but then it's harder for others to leave notes on the future, WIP, steps.
A common situation is that during code review I create a few alternative WIP changes to communicate to a reviewer how I might resolve a comment; they can do the same, and share it with me. Discussion can fork to those change sets.
Gerrit is much closer to my desired workflow than GitHub PRs.
From a continuous integration perspective my understanding is that stacked pulled requests do not make change more frequent if we define a "change" as being committed on the master branch. They only split the feature branch into smaller chunks. On the other hand, I do take your point about context over a number of consecutive changes.
But, to me, "creating a few alternative WIP changes to communicate to a reviewer" indicates an issue with code reviews. I don't think code reviews are the time to propose alternative implementations, even if you have a "better" idea unless the code under review is broken.
What’s your counter proposal on the workflow? No code reviews?
I’m confused. How do you do frequent small changes and avoid stacked PRs. Do you just do a small commit, wait for a review, merge, do another small commit? Or do you make a bunch of small commits locally and only put up the next one for review when the previous one is reviewed and merged?
That’s the only models I can think of and it’s weird to advocate to have a variable time asynchronous process in the middle of your code or review loops. Seems like you’re just handicapping your velocity for no reason.
Stacked PRs are precisely about factoring out small changes into individually reviewable commits that can be reviewed and landed independently, decoupling reviewer and developer while retaining good properties like small commits that the reviewer is going to do a better job on, larger single purpose commits that the reviewer knows to spend more time on without getting overwhelmed dealing with unrelated noise, and the ability to see relationships between smaller commits and the bigger picture. Meanwhile the developer gets to land unobtrusive cleanups that serve a broader goal faster to avoid merge conflicts while getting feedback quicker on work while working towards a larger goal.
The only time stacked commits aren’t as useful is for junior devs who cants organize themselves well enough to understand how to do this well (it’s an art you have to intentionally practice at) and don’t generally have a good handle on the broader scope of what they’re working towards.