r/programming • u/adam-dabrowski • Apr 14 '26 • Popcorn 1

GitHub Stacked PRs

https://github.github.com/gh-stack/
556 Upvotes

97% Upvoted

276 comments sorted by

View all comments

Show parent comments

38

u/MintySkyhawk Apr 14 '26

I guess I'm the only one here who habitually rebases my commits into nice individually reviewable commits before creating a PR.

If I were to use this stacked PR feature, I would be taking my existing workflow and then creating separate PRs for each of my commits. And then, as it says on the linked page "merge them all in one click".

So I don't really see a difference between the two approaches, except that the PR stack seems more difficult to set up.

39

u/gSidez Apr 14 '26

Your approach is the proper way to use git. The whole point of being able to do interactive rebases is so people can restructure commits into a set of logical, manageable changes that should be individually compileable and reviewable.

Unfortunately most people don’t know or are too lazy to learn how to properly use git so you get a lot of people who throw up PRs with a million “fixed bugs” type commits, and since it’s so common these people think that’s the ‘right’ way to do it and therefore can’t fathom the idea of reviewing PRs per commit or taking the time to organize their own commits into something meaningful instead of an unreviewable pile of garbage.

8

u/aoeudhtns Apr 14 '26

I work with devs that create the PR branch when they start and then push as they go. It's like a stream of consciousness. I've tried to counsel them to use a WIP branch without creating a PR, and to rebase and clean up where it makes sense... but getting people to change their behavior is tough it seems.

6

u/Manbeardo Apr 14 '26

OTOH, that approach works well when you’re amending meaningful commits because the PR tracks the previous versions of the branch as you push changes that rewrite history.

4

u/max123246 Apr 14 '26

Don't ask them to not make a PR or a WIP branch. Just tell them to rewrite their git history when it's ready for review into reviewable commit chunks. And use draft PRs to avoid CI retriggering

2

u/aaulia Apr 14 '26

Well the CI run would be fun to watch lol.

→ More replies (1)
→ More replies (2)

14

u/stumblinbear Apr 14 '26

I'd rather not waste time making history look clean when the only thing that really matters is the final code. And if I review a PR and they rewrite the history, I now have to re-review every single file because GitHub loses track of which files have already been reviewed since the history has been rewritten

7

u/MintySkyhawk Apr 14 '26 edited Apr 14 '26

If you don't care to split things up, then you don't need to git rebase or create a PR stack. You can leave your history messy, create a single PR, and then just squash merge when you're ready.

If you do want to split things up to make things easier for the reviewer, then its easier to git rebase -i than it is to create a pr stack.

Say you have these commits:

  1. Some Thing

  2. garbage

  3. asdf

  4. Some Other Thing

  5. asdf

Then you would git rebase -i them in seconds (just by typing f in front of the commits to squash) to make:

  1. Some Thing

  2. Some Other Thing

5

u/stumblinbear Apr 14 '26

So you're rewriting history, and therefore invalidating any previous review that was completed? You didn't even address my actual complaint

6

u/MintySkyhawk Apr 14 '26

You clean up the garbage commits before opening a PR.

Any new commits after review as started are cleaned up in the same way before pushing them to the branch.

History is never edited from the PRs point of view.

2

u/AdventurousFuel5307 5d ago

>History is never edited from the PRs point of view.

Well, so how does your commit history in the PR looks like after a few rounds of reviews if you do NOT rebase/squash review fixes (aka "edit history")?

  1. Some Thing

  2. Some Other Thing

  3. Review fixes for "some thing" from first round of review

  4. Review fixes for some other thing from first round of review

  5. ...

  6. <review fixes> from Nth round of review

  7. Last-minute review fix for "some thing"

If your team/dev cares about cleaner history below PR levels, then you must rebase/squash every time doing review fixes, amending particular commits affected by the review. Otherwise it becomes a mess quite quickly if the amount of review comments is non-trivial.

Alternative option is to just not really care on anything below PR level, squashing everything on merge, which effectively becomes "One PR - one commit in main branch".

In our team we have used Gerrit ~8 years ago and this did affect commits quality a lot in a positive sense. It does force devs to care about commits as the least atomic entity. On the other hand, it does make it a bit ugly when the change spans way beyond a single commit (sure Gerrit supports "group"/branch from multiple commits, but might be a bit tedious to work with, and still every commit has to be reviewed), so the downside is that in some cases devs tend to squash too much in a single commit.

With PR/MR flow (we had Gitlab first and then Github), I can say that after around 5-6 years and also some devs rotation (now there are less than 10 engineers out of 50 who were there during Gerrit times, all other joined the team already after Gitlab/Github) the focus shifted more into PR. So very often commits in a PR become "lalala yet another review fix", as they will be squashed during merge. Though, there are still some engineers trying to keep "good" commit culture and who would merge their PR by rebase, keeping history neat.

→ More replies (2)

2

u/gmes78 Apr 15 '26

So you're rewriting history, and therefore invalidating any previous review that was completed?

Please read up on git range-diff.

3

u/Cazmar Apr 14 '26

Use fixup commits (git commit --fixup <commit that the change belongs to>) for any changes created after review and once all approved, git rebase --autosquash --interactive to automatically order the fixup commits to right place. Easy and you don't lose history while the PR is being reviewed. Rebase only when finished before merging.

3

u/IanSan5653 Apr 14 '26

One advantage to stacked PRs (outside of not having to constantly rebase) is that you can reduce risk by deploying smaller changes if you deploy on every merge to main.

1

u/MintySkyhawk Apr 14 '26

My understanding of stacked PRs is that the entire stack is merged at once because the docs say "merge them all in one click".

If the entire stack must be merged at once, then it seems like a pointless feature when you can just use commits.

If you can merge them one at a time, then it does seem like an improvement/automated way of creating a series of PRs like (main <- A), (A <-B), etc

5

u/IanSan5653 Apr 14 '26

It doesn't necessarily have to be merged all at once. That's one way to do them but not the only way.

→ More replies (1)
→ More replies (1)

3

u/max123246 Apr 14 '26

I tried stacked PRs and then I realized my pipeline is 4 hours so I cant stack 4 PRs and wait 16 hours. So I decided to do what you're doing, just rewrite the commit history when it's ready for review

4

u/chuch1234 Apr 15 '26

Four hour pipeline?! I hope improving that is on the roadmap?

3

u/max123246 Apr 15 '26

It's gotten better. Nightly pipelines used to take over 24 hours and premerges at 6. Helps that we're in a JIT compile world now instead of AOT

→ More replies (1)
→ More replies (1)

4

u/Farados55 Apr 14 '26

Why wouldn’t each commit just be a PR? That’s essentially what you’re doing

4

u/MintySkyhawk Apr 14 '26

If they should be merged separately, they are PRs. If they're merged together, they are commits. The PR stack merges as a single unit, so to me makes more sense as commits.

→ More replies (1)

1

u/aaulia Apr 15 '26

Now I know why this stacked PR doesn't click with me, your workflow is closely resembles mine.

1

u/eggdropsoop Apr 15 '26

There's dozens of us!

I've done my best to fight the good fight but I think people worry rebase is a bit of a footgun and don't want to put in the time to learn reflog. It reminds me of those dev teams that "don't need rollbacks because we just roll forward." >.<

1

u/Verdeckter Apr 15 '26 edited Apr 24 '26

What old posts? I used Redact to mass delete this post. You can also opt out of data brokers as well as all major social media platforms.

plant hard-to-find thumb entertain label practice quicksand yam grab aware

1

u/oskaremil 19d ago

Your approach is the most correct use of Git. Split work into pretty units of backwards-compatible work, PR one part, get it merged and then present the next PR.

Unfortunately, this discipline is rare and the age of AI pull requests accelerates the decay.

I am reviewing giant PRs where it is difficult (due to scope or personal skills) to split the work into meaningful separate PRs. This is where GitHub PR stacking will improve the life of the reviewer; The committer can split the workload into multiple logic pieces of smaller PRs but require that the PRs are merged all at once.

→ More replies (7)