[Okkef]

What's the benefit of this type of stacked PRs over a well-curated set of commits, and reviewing per commit?

I think the bigger problem is that big AI PR's need a different way of reviewing. For example, the order in which the diff's are shown can make a big difference in how easy the commits are to read (e.g., function definition change first, then all call sites, then the tests).

Or maybe we should go to a system where diffs & comments are intertwined, a bit like how "Literate Programming" intertwines code and prose.

Literate diffs / literate pull requests... I haven't found anything like that yet.

[dastbe]

> What's the benefit of this type of stacked PRs over a well-curated set of commits, and reviewing per commit?

For the people who work with stacked diffs (in phab/otherwise) this is exactly what they'd consider reviewing a well-curated set of commits one-by-one.

One distinction is that cognitively a unit of review (a PR, a diff) remains a single bound change. Comments are focused on that change and the PR does not grow with size of the feature

Another distinction is the ability to focus each part of the stack to a particular audience. One change may require review from an external team, another may be just your team mate, a third might be the consuming team. By focusing the stack to the different reviewers you can avoid ambiguity about "what a person is signing off on" in the stack.

aside: one thing that would be great for github reviews is the adoption of change ids such that comments persist across reviews with a rebase workflow.

[skydhash]

IMO, in a team settings, improving the review policies and speed has a much better benefit. A PR is supposed to be a proposal for some change, adding more proposals on top of something that is not reviewed is a bit icky.

> . By focusing the stack to the different reviewers you can avoid ambiguity about "what a person is signing off on" in the stack.

That can be easily done with comments. If the PR are orthogonal, they could have been split. And if they're not, I would really like to know how the part that I'm reviewing interacts with the rest of the changes.

[a_t48]

The PRs may be orthogonal but still be dependent. Feature X depends on improvement Y which also needs bugfix Z. You might go and implement X in a branch, tweaking the codebase as you go, but split the branch apart for review. You put X/Y/Z up, but X contains Y and Z, which means you can't request reviews for X without Y and Z merging, or else have a bunch of extra code that gets in the way.

[skydhash]

Let's say that Z has an error (some assumption that does not hold), and needed to be reverted. How does that impact X's viability? I wouldn't trust any reviews of X after that.

I strongly believe that PR should be compared to the main branch, and not rely on unmerged code. Unless you merge everything together in one go. And in the latter case, everything should be reviewed together.

[a_t48]

I think the answer is - it depends! This is why we make good money. I don’t think there’s a hard and fast rule here to apply.

[Degorath]

My experience is that in companies that rely on this flow, these assumptions are generally hashed out during design doc review/discussions. I have never had it happen to me that I need to discard the rest of the stack for an assumption that did not hold. From another angle, if I fear that this might be the case, I hold off on sending parts of the stack to review until I am sure of the assumption holding OR I instead have a "workspace ball of mud" from which I can `split` out well-formed commits. I found this to be an incredibly productive workflow.

[makeitdouble]

If the tooling allows rollbacking X, Y and Z as a set, it could make sense ?

I'm also on the "compare to the main branch" camp in general, but will sometimes end up with a set of X,Y,Z branches that have different purposes but all depend on X.

More often than not, the base dependency is a set of constants or additional class/methods that could be released with no impact (no reference in live code) but still need a somewhat lenghty review process. Reverts would be happening on the higher level PRs, which hopefully are independent.

[dastbe]

> That can be easily done with comments. If the PR are orthogonal, they could have been split.

Comments are ad-hoc and don't scale, relying on the author to interpret and adhere to the extent of the reviewers approval.

> And if they're not, I would really like to know how the part that I'm reviewing interacts with the rest of the changes.

you are free to look up, down, and around the stack; nobody is hiding the code from you. But in many cases this is just unnecessary.

[tdeck]

This articulates a source of unease I was having trouble pinpointing. Encouraging piling up changes like this puts pressure on the reviewer to approve things as-is because "i've already built Y and Z on top of X so changing X now is a pain".

In my opinion there's already too much of a bias toward stamping PRs, and organizations that maintain high standards do it partly by resisting that bias in key parts of the codebase so people can push back on things that are poorly engineered or hard to maintain.

[skydhash]

I'm strongly on the camp that one PR review shouldn't take more than 10 minutes (vibe number) to review, including reading a linked design spec or speaking with the author for more clarification. Even if I wanted to pull down the branch to test locally, that shouldn't add more than 2 minutes.

Anything that would take a bit longer is either a failure of communication (no design specs, no PR description), a failure of tooling (setting up the project, running tests,...) or a lack of policies (no big changes in a PR).

[hypendev]

But couldnt you just create a PR2 that targets your PR1 branch? What does stack contribute that multiple PRs targeting each other dont?

i.e. like this where each one targets prior PR1 db_migrations PR2 backend_base PR3 adds_frontend_scoobydoodle PR4 adds_some_landing_page_thingy PR5 probably_have_to_fix_something

Or is stacks just a feature built to make this type of PR stacking easier to review?

[m11a]

In essence. But if you have a chain PR1 > PR2 > PR3, and PR1 gets merged, all the others (ime) seem to not cleanly rebase on main. They end up with conflicts that require manual fixing. I've not really figured out why, tbh. It'd also be nice to see a coherent link in the UI between PR1, PR2, PR3.

[eddythompson80]

> Or is stacks just a feature built to make this type of PR stacking easier to review?

Pretty much, yes. It’s just a UI to make it clear that those 5 PRs are liked (stacked) and gives some tooling around reviewing, merging and managing them, reduce redundant CI runs if desired, etc

[teeray]

It’s the “well-curated” part. Many folks treat commits like video game save points, and find providing any kind of message burdensome (e.g. “fix bug”, “do work”). This alone is fine, but then they can’t be bothered to go back and clean it up with `git rebase -i`, so you end up with a mountain of trash in your git log if you don’t turn on mandatory squash and merge. For these folks, the PR becomes the commit. Stacked PRs is revolutionary because it’s as though these developers can finally have multiple commits which comprise a change.

[ghthor]

This is it, but also, because there was no way of asking a reviewer to review a single commit and then merge only that commit out of 2+ commits without a lot of ceremony, work streams had to be completely orthogonal to parallelize anything . Now I can fully develop a sequence of 20 changes (each a single commit) and start getting reviews and merging while still progressing towards the goal. I’ve been doing this for 2years with graphite and more before that manually or in other systems/cultures that actually can review a commit/patch and merge that thing.

Honestly all GitHub needed to do was make reviewing and merging commit(s) from a PR possible and that would also been neat.

[chillfox]

There is real value in having all the messy commit history, especially now with AI, but you do need some kind of description on them that indicates what they are.

One use I have got from it is asking an agent to go through the git history and categories the mistakes/bugs, then turn the common ones into CI checks or AGENTS.md rules.

A well curated history just hides a lot of valuable info.

[eddythompson80]

> (e.g. “fix bug”, “do work”)

or "skjdnfks" and "fsdfs" commits.

[dualvariable]

> What's the benefit of this type of stacked PRs over a well-curated set of commits, and reviewing per commit?

What I can see is that you can easily append commits to e.g. the first PR in a stack, which would insert them into the middle of sequence of commits.

This will require rebasing and fixing the subsequent PRs in a stack the same way you'd need to rebase and fix the subsequent commits in a mega-PR. But it makes the right thing easy (keeping all the commits to the foundation of the change together) rather than makin the wrong thing easy (appending fixup commits across the entire change in a random order so that the actual foundational change is lost).

Keeping all the foundational commits together also keeps all the discussion over the foundational change together.

You could argue that you'd want to only do the foundational PR and stop, but doing the whole stack of PRs gives the reviewers more information about where you're going, and allows work to continue asynchronously.

[Cedricgc]

With stacking you can continue working on a longer change while creating reviewable diffs. Merged diffs can be rebased on your HEAD. For teams that support stacked diffs you don't usually even need to manage branches and work directly off trunk and rebase changes as they come in

[ahepp]

I don't have experience with the stacked PR workflow and often find myself asking the same questions you are.

As far as I can tell, the biggest benefit of stacked PRs over just making a coherent series of commits, is that it might make it easier to start work on your second PR before you merge the first one?

With human-in-the-loop coding, that sounds like it could lead to a lot of wasted work if the first PR gets substantial feedback. But with agentic coding, I can imagine how it might be desirable to keep the agent chugging while the first PR is under review.

Interested in learning more about it and generally agree that AI is stressing the current review paradigms a lot of us are accustomed to.

[332451b]

For me it's hard to imagine not starting on the second PR after submitting the first one, regardless of AI or stacked PRs. For focused work I find it important to avoid context switching, stay in the zone and keep everything in my head. Wasted work in comparison feels less significant to me.

If review is fast I'd be switching back and forth between tasks throughout the day. If review is slow I might end up implementing a feature over the course of two weeks rather than two days. With reviewers in different time zones, limited time or doing their own focused work, it adds a lot of latency going back and forth for every PR rather than iterating on a stack of PRs.

Personally I find the biggest benefit is that it lets both the author and reviewers work at the pace that works for them, with reduced context switching and latency.

[ghthor]

If you make good commits and each of them is a valuable unit of work; having tooling that can manage rebasing them and creating a reviewable PR for each is super valuable for maintaining momentum and making reviews easier.

[paxys]

You can’t merge one commit at a time in a PR. In a stack, if the first 4 parts of a feature are good to go and there’s a problem with the 5th, the whole thing doesn’t need to be blocked.

[ahepp]

I wonder if they could have added the ability to take only certain commits from a PR though?

It kinda seems like they're duplicating the "unit of change" arbitrarily, rather than just fixing the way a PR works.

I think the reason I find that a bit icky is it seems like it's diverging GitHub from the underlying git tool, which I trust a lot more.

[mcintyre1994]

The issue you’d have then is the CI might not be green after the 4th commit but it is after the 5th. A single PR will hide that, so you’d need a way to run the CI on the subset of commits you want to extract. It’s also much easier to isolate those blocking comments on the 5th commit if it’s in its own PR and not mixed with all the review comments on the other ones.

I don’t really see it as diverging much from the underlying git tool TBH - it’s still just git branches pointing at each other.

[paxys]

It isn't arbitrary. A branch is the unit of change. You can stack branches on top of each other and merge them one at a time. You can also keep adding to the end of the stack as needed.

[ahepp]

I understand that GitHub kinda ate the world on this, but using a branch PR as the basic unit of change seems like an arbitrary choice to me.

Torvalds invented Git for work on the Linux kernel, and the basic unit of change there is commits over email. It's not uncommon for them to take some commits but reject others.

Which isn't meant to say "pull requests are obviously wrong and commits over email are obviously correct", but rather to point out that the idea of a take-it-or-leave-it branch isn't some kind of natural unit of contribution

https://lkml.iu.edu/hypermail/linux/kernel/1402.2/00508.html

https://lkml.indiana.edu/2011.2/05416.html

https://lkml.iu.edu/2409.0/00960.html

https://lkml.iu.edu/hypermail/linux/kernel/2510.3/01369.html

[skydhash]

> Git for work on the Linux kernel, and the basic unit of change there is commits over email. It's not uncommon for them to take some commits but reject others.

After reading this article[0], it's become clear to me that what you want is a reference branch, which people propose changes to. Then there's the integration process that takes a unit of change (commit or PR) and then tests that the reference branch is <good> after applying that change. If it's not, then that unit of changes is sent for rework. If it is, it's now become part of the reference branch and the next units of change needs to be able to work together with it.

In corporate work, I tend for the PR to be the unit of work, which means squashing and merging. But any PR is compared to the default branch, not each other. Anything that is not yet part of the default branch, even if approved, is unreliable. So as the author, you'll need to refresh your PR to not rely on an old state of the reference branch.

I think there is too much focus on the PR itself instead of the reference branch which reflect the true state of the next release. Like their PR was good at the time of writing, so it should be merged. Instead of whether merging their PR would break the reference branch.

[0] http://kroah.com/log/blog/2019/08/14/patch-workflow-with-mut...

[Am4TIfIsER0ppos]

> You can’t merge one commit at a time in a PR

Sure you can. The command is `git merge`

[mmlb]

I've had that same "what's the point" thought every time I've read about stacked PRs, but recently had an (obvious) epiphany. The benefit is you get CI for each commit! I've always hated fixup/typo/fix tests commits and toyed with having a CI check that enforced ci passing on each commit but this drops that need.

[satvikpendem]

I don't get it, we already have CI for each commit, at least at our workplace, enforced.

[tabwidth]

Other side of that though, touch the bottom of a stack and every layer above it re-runs. Stack unwinding, basically.

[skydhash]

> I've always hated fixup/typo/fix tests commits and toyed with having a CI check that enforced ci passing on each commit but this drops that need.

Can't you run your CI locally? I know it's not feasible for some codebase, but at least the linting, formatting, unit tests, some integration tests should be able to be done locally.

[nijave]

>Can't you run your CI locally?

If you mean automated tests and linters, sure.

Conceptually continuous integration is generally integrating 2+ different lineages of code together which is more common with multiple developers although I suppose it's becoming more relevant with agents creating a bunch of worktrees with different things.

In practice, CI has taken the same path as "DevOps Engineer" ie most people just mean "automated test server"

The opposite of CI is more-or-less merge windows or merge-fest like Linux where everyone mails in their changes and someone manually integrates.

[happimess]

> Conceptually continuous integration is generally integrating 2+ different lineages of code together

> In practice, CI [often means] "automated test server"

Could you say more? I use "CI" to refer to the automated processes that run tests and (maybe) deploy code as it is merged to some blessed branch. It's what continually integrates the new code into the existing code. What am I missing?

[nijave]

If there's 1 developer working on 1 branch at a time: main is at commit A, you branch add B and C, you merge now main is at A, B, C. Your "integration" was just moving the code forward and if you ran tests on A, B, C commits and it passed, the tests would still pass on main.

Now say there's 2 developers that branch off A. Dev 1 removed dead code coolUtil and with it deleted module coolLib and commits B1. Tests pass, code works. Dev 2 adds new coolUtil2 in commit B2. Tests pass, code works. Now they both merge their changes back to branch A so it's A, B1, B2 or A, B2, B1 or A, B3 (merge of B1/2). Anyway, you try to run the code and it fails. Everything merged clean but Dev 1 removed the lib Dev 2 depends on. They didn't create any file level conflicts because they edited different things so the merging went fine however now the code is broken.

The real integration part comes from reconciling different (potentially conflicting) changes and making sure the end result still works. The server is doing the integration automatically and verifying it automatically

You could skip the server and merge everything on your machine and run the tests on your machine and achieve the same result. Still CI, no server, no automation

Hopefully that makes sense, if not I think the Wikipedia page "Practices" section does a decent job separating the pieces https://en.wikipedia.org/wiki/Continuous_integration

[steveklabnik]

You can't have mac/windows/linux/whatever all locally simultanously.

Not every project requires this, but for those that do, it's impossible.

Also, it is much harder to enforce "everyone must run each commit through the CI equivalent properly" than it is when it's on your forge.

[skydhash]

> You can't have mac/windows/linux/whatever all locally simultanously.

Why can't you? That's what VMs are for. And even then, most cross-platform codebases have an abstraction layer that rarely changes. So even testing on one platform can raise your confidence very high.

> Also, it is much harder to enforce "everyone must run each commit through the CI equivalent properly"

Again why? I wouldn't care about the dev's local branch. But what is send to the main repo can be easily scripted to run the CI on every commit. You just send the result back with each commit that fails. They can replicate the same workflow on their local workspace as a pre-push process.

[steveklabnik]

You can't run MacOS in a VM on non-Mac hardware without violating Apple's EULA.

> But what is send to the main repo can be easily scripted to run the CI on every commit.

Sure, this could work. I didn't say it was impossible, just more difficult. You have to build all of this support on top of the system that already does it for you: have your forge run CI on every commit.

[sfink]

Heh, it's a fun reminder that different people work on different things.

Sure, I could load up my build machine with mac/windows/linux/android, with multiple versions of each. Emulating x86+x86_64+arm32+arm64+aarch64+... is also doable, sort of.

But considering that the real CI runs several thousand hours of tests total across all platforms, I think I won't.

Also, my Electron-based IDE needs those 10s of GB to edit text. How can you possibly edit a 1KB text file with less than 1GB of RAM?

Not to mention clangd that needs to do its ultra-important work Right Now so I can invalidate it all with my next edit. That's probably the biggest RAM hog of them all.

[m11a]

GitHub's PR workflow doesn't nicely support being able to review individual commits, realise which comments are associated with which commits, etc. Or shipping individual commits to main, while working on some others (unless you allow cherry-picking and direct push to main). Or amending a certain commit with respect to feedback and seeing a diff from the previous patch of that commit to the next.

If GitHub's unit of change were a diff, and not a branch, then that would work pretty well.

[othmanosx]

have you tried pyor.review? it is actually just github but with better UI, and you get a commits rail that makes it easy to switch between commits on the fly

[EduardoBautista]

I feel it's more of a limitation of the GitHub UI. It is much easier to group together reviews and comments by PR than it is by commits.

[ghthor]

Yep, this exactly

[aeturnum]

I would say that it makes it easier to design a PR and then go back and update that PR as things evolve. For instance, you might decide you need one table so your first PR in the stack adds that table. But then you finish up with 2 or 3 more PRs and you get reviewed and you and your colleagues decide to add a 2nd table. Now you get to go back to that first PR, add the 2nd table and then you get to cleanly review that 2nd table. Then each PR in the stack also gets a clean small addition to handle the table, that can be reviewed in its own context with smaller diffs.

So the idea is you can much more cleanly isolate changes for large features.

[brabel]

I’ve been using stacked prs for many years, AI did not change anything in that area. The advantage is that you can control what lands in the trunk independently of what a particular review contains, and that you can commit freely without having to later go back and do archeology on past commits to make them look reviewable. The unit of commit and the unit of review is not the same and you shouldn’t try to make them so, they have entirely different purposes.

[madeofpalk]

it's basically a UI for a well-curated set of commits, and reviewing per commit.

[lnrd]

> What's the benefit of this type of stacked PRs over a well-curated set of commits, and reviewing per commit?

Even if the reviewer does the review commit-by-commit, all the comments and discussions will be on the same PR leading to multiple ongoing conversations about different topics that would be split if the PR are stacked. Also, all the new commits addressing the comments with spoil this commit-by-commit design, as the previous commits will be outdated and the new changes will be on top of those. I think it's beneficial for new changes to be a separate commit and not rewriting history, to not force the reviewers to re-read everything but just the latest changes.

[IshKebab]

1. Github's UI doesn't really support reviewing individual commits in a big PR.

2. It also can't merge subsets of commits from a single PR. E.g. if you have two commits, A and B, where B depends on A... sure you can make a PR containing A and B, but if A gets approved and B doesn't, then you can't merge A.

3. The thing you want to do with a set of commits and reviewing each commit IS stacked PRs.

This is nothing to do with AI.

[hoppp]

I think it is designed for AI Pull Requests. My impression was that it's for reviewing large generated changes.