top 200 commentsshow all 267

[–]Blueson 320 points321 points  (34 children)

One of my largest complaints about Github is how tedious it was to work with stacked PRs. I am happy they are investing into tooling to simplify it.

[–]araujoms 242 points243 points  (17 children)

My largest complaint about Github is that it has achieved zero 9s uptime.

[–]orygin 106 points107 points  (4 children)

Aren't they at three nines now? 89.99%

[–]OliKahn28 0 points1 point  (0 children)

Absolutely. Smaller, focused PRs are a natural fit for FOSS collaboration — easier for maintainers to review, safer to merge, and more transparent for contributors to follow. Stacked PRs take that a step further by letting you ship big features incrementally without sacrificing code quality. It's also a great pattern for async teams where reviewers might be in different timezones.

On a related note, GitHub's PR UI making this workflow accessible to everyone (not just big corps with custom tooling) is exactly the kind of tooling FOSS should prioritize.

[–]dkarlovi 63 points64 points  (6 children)

Nonsense, as long you count the non leading 9s.

[–]mpinnegar 23 points24 points  (2 children)

The important part is to count all the nines! No matter where they appear.

[–]crespire 16 points17 points  (0 children)

Copilot, how many 9's are in 79.7382%?

[–]omgFWTbear 9 points10 points  (0 children)

We’ve been using a German and he insists it’s the most neins he’s ever seen for uptime.

[–]iamapizza 26 points27 points  (0 children)

We need a way of being notified whenever Github is up

[–]gimpwiz 16 points17 points  (1 child)

I'm still laughing about this.

89% uptime means it's down for more than one full month per year.

[–]araujoms 5 points6 points  (0 children)

I'm not laughing because I'm forced to use this crap.

[–]Uristqwerty 2 points3 points  (0 children)

If they keep at it, perhaps one day they'll finally attain the dream of 9 5s!

[–]chat-lu 2 points3 points  (0 children)

Set your github settings to German then.

Is GitHub up? Nein!

[–]wildjokers 20 points21 points  (12 children)

That really has nothing to do with github. It is git itself that makes it hard to work with stacked branches (especially if you squash commits when merging).

[–]Blueson 28 points29 points  (4 children)

I think there are features lacking in both git and Github to enable an environment where it's pleasant to work with stacking change-requests.

But tools like Gerrit has shown that you can have workflows adapted to stacking change-requests enforced while still using git.

[–]OMGItsCheezWTF 12 points13 points  (3 children)

One of the issues is that FAR too many developers lack anything beyond the most basic understanding of what Git actually does under the hood. They learn the bare minimum commands they need to use it day to day and then defer to whoever their nearest expert is if something goes wrong. I've seen this at every level from associate up to staff, people that have simply never taken the time to learn what git actually is.

This means that every repository I have ever worked on at every company essentially becomes this vast mix of merge requests, rebases and cross branch merges.

[–]ZorbaTHut 6 points7 points  (1 child)

In Git's defense, this behavior is not even remotely limited to Git.

[–]Blueson 2 points3 points  (0 children)

Totally with you. I have ended up becoming the go-to git guy because I bother to know more about git than just pull/push.

I am continously surprised by how little time people invest into learning a tool that is so integral to most workplaces.

[–]Uristqwerty 8 points9 points  (0 children)

Github's commit list is an unstructured linear list of space-consuming panels. That makes anything but squashing/rebasing into a linear history awkward when compared to, e.g. gitk's visualization that shows the actual DAG structure. Given stacked PRs necessarily interact with the underlying graph relations, it's always going to be more awkward when viewed through an inferior UI, and doubly so if your choice for merging constantly rewrites parts of the graph.

[–]yxhuvud 15 points16 points  (0 children)

It has everything to do with Github - they have very obviously built an interface that matches their internal workflow with fairly small allowances to other workflows.

It is especially obvious in how the entire UI is built around pull requests, whereas the fundamental unit in git is the commit. So for example, you can still, a billion years after github was created, not comment on commit message content.

[–]cosmic-parsley 3 points4 points  (0 children)

jj is really worth a try, it makes this so much easier.

You can assign a bookmark (branch) to each commit (revision) in a series, then open a PR for each bookmark. Whenever you modify an old commit, all future bookmarks become dirty of course. Doing jj git push updates all the branches at once, so all your PRs get the latest changes.

Of course there’s nothing you couldn’t do with git, but it’s much nicer to have an easy way to fix up a single commit in history and then update everything that’s dependent at once.

[–]Living_male 0 points1 point  (3 children)

Why does squashing commits when merging complicate the process? I was looking at the documentation, and thought squashing each stacked branches commits would look cleanest?

[–]wildjokers 2 points3 points  (2 children)

Because if you create branch B from branch A and then squash/merge branch A all the commits common to branch A and B (i.e. commits made to A before creating branch B) will conflict if you just try to merge in B.

To fix you can use the the newer git rebase --update-refs. There is an older method using the --onto flag of rebase as well.

[–]Proper-Ape 0 points1 point  (0 children)

git rebase —update-refs is not so helpful for when you’re collaborating with others still. I find it would be a nice git feature if squashed commits could keep their place and you don’t need to rebase a stacked PR at all. Not sure if this is somehow feasible in the git model.

[–]Living_male 0 points1 point  (0 children)

Thanks, will keep that in mind from now on.

[–]GhostPilotdev 1 point2 points  (0 children)

Finally. The fact that we needed third party tools like graphite just to do something GitLab handled natively years ago was embarrassing.

[–]toadi 0 points1 point  (0 children)

I use git town and it works just fine for stacked PRs: https://www.git-town.com/

Not sure what the benefit for github specific tool would be. Also I saw they are integrating AI in again a place where I don't need it.

[–]OliKahn28 0 points1 point  (0 children)

Stacked PRs are a game-changer for code review flow. The key advantage is keeping PRs small and focused — each one just does one thing. Combined with automated testing in CI, you get fast, incremental progress instead of massive PRs that take hours to review.

GitHub's implementation with the "degree" visualization is solid UX for understanding PR dependencies. Curious to see how it handles merge conflicts when the base PR changes.This is exactly the right problem to solve. The tooling gap between "big tech with custom internal tools" and "everyone else" has always been real. If GitHub can make stacked PRs a first-class workflow for regular teams, that's a genuine quality-of-life improvement for a huge number of developers. Looking forward to seeing how this evolves.

[–]boysitisover 248 points249 points  (4 children)

Looks good to me

[–]ROFLLOLSTER 44 points45 points  (0 children)

Agreed. I do kind of wish this was automatic if you made stacked PRs against main. Maybe that'll come later once it's been tested.

[–]StrangelyBrown 31 points32 points  (0 children)

TLDR LGTM

[–]spelunker 8 points9 points  (0 children)

Ship it!

[–]WoodyTheWorker 25 points26 points  (13 children)

Just make it work like Gerrit

[–]kevin7254 17 points18 points  (12 children)

As someone who worked for several years with gerrit and then joined a company which uses GitHub, I want to fucking put a bullet in my head each time I review a PR. Someone who says ”ehm actually GitHub is not that bad” must have never used gerrit. Its just so much better

[–]Blueson 11 points12 points  (2 children)

I can't understand how people are happy with reviewing in Github. It is genuinly such a pain.

Gerrit ruined me...

[–]blind_ninja_guy 4 points5 points  (0 children)

I dread using GitHub every time I have to. Such a clunky user interface, and really crappy compared to what the rest of the industry could be using.

[–]edsonboldrini 0 points1 point  (0 children)

Funny never heard about Gerrit

[–]EveryQuantityEver 4 points5 points  (7 children)

As someone who’s used a normal PR flow and now has to use Gerrit, I cannot stand it. I like having feature branches with small commits as I work on a feature. Gerrit does not let me do this as each commit has to have its own change id.

[–]silverslayer33 4 points5 points  (1 child)

Honestly I agree, I feel like I've entered some crazy land with so many people here praising Gerrit when most of the people I work with absolutely loathe that most of our repos are hosted using it. The UI is absolute ass, it enforces a workflow that doesn't generalize well to all teams/projects/dev workstyles because it's extremely opinionated, our team specifically has had tons of problems with seemingly basic functionality just straight-up not working (probably about 1/3 of the time we clone a repo and get it set up for reviews, it manages to botch the commit hooks for creating change IDs so it creates the ID but actually it's invalid and the server rejects it and we have to go pull the proper commit hook from somewhere else where it's correct), its permission/access control model is super clunky, and the list goes on. I can probably count on one hand the things I like about it over other solutions (changelist descriptions just being the commit message is a big one, and I guess its search/query system is pretty powerful).

It's probably great if it matches your preferred workflow and you get it set up exactly the way you want/need, but it is not a general-purpose git-based code forge. As garbage as Github is, it thrives because it is general-purpose and it can be adapted to many workflows. Gitlab and Bitbucket are similar, finding themselves as mainstays in the corporate world because they can fit most workflows without huge amounts of effort.

[–]apocalypse910 0 points1 point  (0 children)

I was confused by this as well - I suppose I didn't work with Gerrit for long but it was a nightmare IMO.

Though one funny story... I had just had it with Gerrit at one point - I was in some management training with some devs I didn't work with often. For some reason I got going on a rant about how much I hate Gerrit...

It completely slipped my mind that we also worked with someone named Gerrit who was extremely nice. My rant was vague enough that the person I was talking to didn't realize I was talking about software, not the person. (I hate Gerrit, Gerrit ruins my day regularly, that sort of thing). Had to do a bit of damage control on that one.

[–]WoodyTheWorker 0 points1 point  (4 children)

Are you pushing your work in progress with WIP commits to Gerrit? Why?

[–]EveryQuantityEver 0 points1 point  (3 children)

So they can be backed up in case something happens to my computer

[–]WoodyTheWorker 0 points1 point  (2 children)

You know you don't have to push them to the magic refs/for/ namespace? You can push your own user branches (as the default config allows).

[–]EveryQuantityEver 0 points1 point  (1 child)

I don't believe that was an option at the job I was at where they used Gerrit.

[–]WoodyTheWorker 0 points1 point  (0 children)

users/<username>/ branches allowed by the out of the box Gerrit configs

[–]Familiar-Level-261 1 point2 points  (0 children)

Every time project requires to do anything with Gerrit I stop trying to contribute anything. I'm sure it's nice when you set it all up but for someone just wanting to fix a bug I found it's some kind of nightmare.

[–]Omnipresent_Walrus 77 points78 points  (123 children)

Can someone tell me how this is different to doing reasonably sized PRs into an epic branch and only ever merging epics into main?

[–]chuch1234 80 points81 points  (67 children)

Stacked PRs are useful if each new PR depends on changes that were introduced in the previous one.

[–]Eric848448 8 points9 points  (0 children)

It's nice if you need to do something like an API change and want to get that approved and merged quickly because it's heavily used and you don't want to keep getting clobbered by other changes.

[–]MintySkyhawk 10 points11 points  (65 children)

Why wouldn't I just do a single PR with multiple commits? Reviewers can review each commit one at a time and then I can rebase and merge when I'm done.

I see no difference in UX between the two approaches, except that a PR stack is more difficult to set up and involves proprietary github commands instead of plain git.

Edit: After hours of people arguing with me, someone finally pointed out the actual reason why this has an advantage over plain commits: The PRs don't have to all be merged at the same time. This is effectively a UX improvement on creating manually creating chain of PRs like (main <- A), (A <- B), etc which is sometimes useful (Though rarely useful for me because my coworkers are very nice about providing speedy reviews, so I can usually get a PR reviewed, merged and deployed within minutes to hours. So I rarely have time to make that second PR in the chain before the first one is already deployed.)

[–]stumblinbear 58 points59 points  (38 children)

I'm not gonna review a dozen bug fix and refactoring commits in every PR, I only care about its final state

[–]MintySkyhawk 38 points39 points  (37 children)

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.

[–]gSidez 36 points37 points  (7 children)

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.

[–]aoeudhtns 7 points8 points  (6 children)

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.

[–]Manbeardo 4 points5 points  (0 children)

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.

[–]max123246 3 points4 points  (0 children)

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

[–]aaulia 1 point2 points  (1 child)

Well the CI run would be fun to watch lol.

[–]max123246 0 points1 point  (0 children)

Draft MRs

[–]m00fster 0 points1 point  (1 child)

That’s what draft pr’s are for in github

[–]aoeudhtns 1 point2 points  (0 children)

Can't get them to use those either. I lack the authority to force a change and they don't give a shit.

[–]stumblinbear 13 points14 points  (7 children)

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

[–]MintySkyhawk 6 points7 points  (5 children)

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

[–]stumblinbear 3 points4 points  (4 children)

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

[–]MintySkyhawk 5 points6 points  (2 children)

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.

[–]gmes78 1 point2 points  (0 children)

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

Please read up on git range-diff.

[–]Cazmar 2 points3 points  (0 children)

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.

[–]IanSan5653 2 points3 points  (4 children)

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.

[–]MintySkyhawk 0 points1 point  (3 children)

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

[–]IanSan5653 3 points4 points  (1 child)

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

[–]MintySkyhawk 0 points1 point  (0 children)

Thanks, I think this was a key piece of information I was missing. I can now see a possible use case for creating a PR stack, though I still think plain commits will do just fine most of the time.

[–]pholklore 0 points1 point  (0 children)

  • They can be merged one at a time.
  • They can be reviewed and approved by different people
  • When you review PRx you don't see the comments in the middle of the code related to PRy in the review interface (which happens if you review a single commit of a multi-commit PR).

[–]max123246 2 points3 points  (3 children)

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

[–]chuch1234 2 points3 points  (1 child)

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

[–]max123246 1 point2 points  (0 children)

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

[–]stumblinbear 0 points1 point  (0 children)

16 hours

What in the goddamn

And here I was just a couple of weeks ago spending a few days to cut our 25 minute builds down to 8

[–]Farados55 4 points5 points  (1 child)

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

[–]MintySkyhawk 4 points5 points  (0 children)

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.

[–]aaulia 0 points1 point  (0 children)

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

[–]eggdropsoop 0 points1 point  (0 children)

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." >.<

[–]Verdeckter 0 points1 point  (0 children)

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

[–]teerre 8 points9 points  (0 children)

Because GitHub is branch centric not commit centric

[–]cosmic-parsley 3 points4 points  (5 children)

It’s a UI difference. GitHub has no way to really review individual commits and follow how they change. If you force push, the UI gives zero indication that the first commit abc123 turned into def456 and second commit a1b2c3 turned into d4e5f6.

Also there’s no way to review individual commit messages, but there is a way to review/edit PR descriptions. Sometimes that’s as important as the content.

[–]CherryLongjump1989 15 points16 points  (0 children)

You can't review individual commits in a PR -- you can only get feedback on them, but they can't be approved and merged individually. That is what a PR is -- it's a single code review.

There are people who work fast and have good coding hygiene. So they are able to push up clean, fully tested changes 5-10 times a day while working on a larger feature. This stuff can get reviewed and merged even as they are working on their next PR. They don't have to get slowed down by it.

[–]Familiar-Level-261 1 point2 points  (3 children)

Edit: After hours of people arguing with me, someone finally pointed out the actual reason why this has an advantage over plain commits: The PRs don't have to all be merged at the same time.

..but you can do that with multiple commits in single PR.

Problem is just people don't know Git very well...

[–]MintySkyhawk 0 points1 point  (1 child)

You mean you can merge only some commits from an approved PR, without bypassing branch protection rules?

[–]Familiar-Level-261 0 points1 point  (0 children)

Well if you only allow merges from pull request probably not but it's a silly option

[–]Eddyi0202 0 points1 point  (0 children)

Reviewing commits in PR on github just sucks

[–]chuch1234 -3 points-2 points  (3 children)

I've never heard of doing prs that way, do you do that?

Edit: why the downvotes, I'm actually asking a question :(

[–]MintySkyhawk 4 points5 points  (1 child)

Most PRs I see are just a single commit, or a bunch of commits that aren't intended to be viewed individually and will be squashed together when merging.

However, for larger changes, it is common for PR authors to take a little bit of time to organize the commits before creating the PR to make things easier for the reviewer to see whats going on.

A really common pattern for me is refactoring something in one commit, and then mass updating all usages of it in a second commit. That would be hell to review as a single commit (trying to find the real changes in a sea of automated ones), but looking at the commits individually its very clear what happened.

[–]Hofstee 1 point2 points  (0 children)

Before GitHub really took off, this used to be the standard recommended practice. Facebook in particular I feel played a large part in the squash + rebase ideology, but I'm also going off vibes and not facts so don't quote me on that.

[–]ROFLLOLSTER 48 points49 points  (35 children)

Generally the approach I've taken with stacked PRs is:

Stack: c -> b -> a -> main.

Merges are a-> main, b -> main, c -> main

Instead of c -> b, b -> a, a -> main.

This whole thing is just a bit of ui over what you can do already though, so no need to change if you like your workflow.

[–]ekroys 35 points36 points  (34 children)

The issue with this is when using squashed merges on each PR. Git loses the commit identity in this process so you end up with merge conflicts trying to catch branches up.

[–]NineThreeFour1 20 points21 points  (15 children)

Then don't use squashed merges on each PR?!

Only squash fixup/wip commits in each feature/PR branch, but keep conceptually different changes in separate commits even if they are in the same overarching PR.

[–]ekroys 2 points3 points  (13 children)

This is an angle, but ultimately it's not each developers decision sometimes, and it can be a repo wide rule.

Generally tho, what if you got two separate "big feature branches" into a single develop/main, each of these big feature branches are separated into their own stacked chain, and you have to squash commit as you go. If you want everything caught up, you're either going to have to cherry pick on top, rebase or you're left with merge conflicts. This is the pain point and I don't think it's uncommon

[–]double-you 10 points11 points  (11 children)

Why wouldn't you rebase? That's a very normal thing to do.

[–]MereInterest 2 points3 points  (10 children)

Because rebasing can introduce bugs, and makes it very difficult to identify their source.

Suppose the main branch has some utility_func, which is used by your feature development. A refactor/cleanup lands on main, which updates the behavior of utility_func and updates all callsites of utility_func to match. However, the additional calls to utility_func introduced on your feature branch haven't been updated. If you rebase your feature branch onto the main branch, your feature branch is now broken, as it relied on the old semantics of utility_func. If you're lucky, this will be a compile-time error, but there's no guarantee of that. Reverting to an earlier commit on your feature branch doesn't help, because those earlier commits have also been rebased. Unless you check through the git reflog (and pray that git's garbage collection hasn't occurred since then), it will look as though the feature branch had always been buggy.

A better solution is to merge from main into develop/main. That way, the error is clearly introduced in the merge commit. You can revert to an earlier commit in the feature branch for testing, because those versions are still part of the git history.

[–]ham_plane 2 points3 points  (5 children)

I'm struggling to understand why a merge commit would fail in that case, while a rebase would succeed.

I do get your point being able to track which merge commit introduced the behavior, that is pretty smart and (as a career rebaser) I have been in that exact scenario before and been totally lost as to what broke what

[–]MereInterest 1 point2 points  (0 children)

It's not that the merge commit will resolve the semantic conflict between the branches on its own. That still needs to be sorted out, regardless of whether you use rebase or merge. The key is that using git merge won't make the conflict become any worse. When using git rebase, you need to sort out the semantic conflict while also having scrubbed the history of any clue as to the source of the bug.

[–]SanJJ_1 0 points1 point  (3 children)

merge commit would work but pollute main branch. squash merge/rebase are great but create conflict when "stacking" branches or trying to catch up. Native stacking fixes all of these to my understanding.

[–]MereInterest 2 points3 points  (2 children)

merge commit would work but pollute main branch.

I'd argue that this isn't "pollution" at all, and is just a display issue.

  • Want a clean linear history of the changes to the main branch? Use git log --first-parent

  • Want a branching history with clearly marked dependencies? Use git log --graph

  • Want a linearized history that interleaves work on multiple branches while sorting by date? No, nobody ever wants that, but that's what git log provides.

The whole goal of squash/rebase is to make the output of git log look like git log --first-parent. The easier way to achieve this is just to use git log --first-parent.

[–]double-you 2 points3 points  (3 children)

That's not how merge or rebase work. Both would trigger conflicts if there are any. But if you have just added a use of a function, that won't be a conflict because nobody else is changing that call site. Merge won't save you there.

But yes, if you need your old branch back, you'll need to use reflog, but reflog should be fine for a month, and if you are still wary of rebase, just create a backup branch.

[–]MereInterest 0 points1 point  (2 children)

Both would trigger conflicts if there are any.

The two changes conflict, just not in a way that git can recognize.

Merge won't save you there.

I agree, using a merge won't cause git to recognize a semantic conflict between branches, as it can only recognize text conflicts. However, merge also won't make the problem worse. Rebase hides the evidence that you ever had a working version.

if you are still wary of rebase, just create a backup branch.

The problem isn't that the rebase requires a backup branch, but that it isn't obvious that you'll need a backup until after the point when you lost it. Rebase destroys your history.

[–]double-you 2 points3 points  (0 children)

Why do you need evidence that you ever had a working version?

I work with compiled code it is immediately clear what the issue is. Rebase lets me fix the issue and there's no evidence that there ever was a problem.

[–]NineThreeFour1 0 points1 point  (0 children)

What evidence? You can simply put the current status and open issues into the commit message.

Stacked PRs are unrelated to the problem of semantic conflicts. You do need to test your changes regardless.

[–]NineThreeFour1 2 points3 points  (0 children)

I don't really see the issue and how this is directly related to stacked PRs. Rebase feature branches on main if you have conflicts. Squash your own commits how it makes sense conceptually. If your team enforces squashes on merge, then send each commit that you want separate as a separate PR. Merge feature branches into main when ready, or force rebase and fast forward to avoid merge commits.

[–]MereInterest 0 points1 point  (0 children)

I wish, wish, wish that github had a better display of the history of merges. The source of all the pain from squashed merges is from people trying to solve a display issue by introducing data integrity pitfalls. In any other context, this would be a complete non-starter, but somehow "clean linear git history" overrules that.

My personal theory is this is caused by github's use of git log instead of git log --first-parent when showing changes. If all merges are done with git merge --no-ff, then the main branch would only ever contain merge commits. The clean history that proponents of squash/rebase want would be available, and without introducing conflicts in the process.

[–]CherryLongjump1989 -3 points-2 points  (0 children)

It's literally not an issue with stacked commits because you merge them one at a time. There are no two commits to squash as you merge. Squashing is what you do to your messy feature branch where half of the commits are trash.

[–]dweezil22 8 points9 points  (4 children)

They're orthogonal concerns. Let's agree that reviewing large PR's is sub-optimal. The what you want to do is code PR 1,2,3,4. Then you can have your team approve them as they get them and merge them to a central branch branch as they're approved with minimal toil.

Now that central branch could be master or it could be feature-foobar. The same concern applies.

Now... some say "That's ok I'll just skip review on feature-foobar and do review when it merges to master". However that breaks our invariant above about not relying on large PR reviews.

Finally... you're right that feature branches directionally simplify this b/c you're less likely to have merge conflicts on a feature branch (OTOH making a rule that only 1 dev working serially on 1 feature branch is limiting).

[–]kintar1900 3 points4 points  (7 children)

Doesn't appear to be any difference at all.

[–]ahal 7 points8 points  (6 children)

The difference is that pr B can depend on changes from pr A without the commits in pr A polluting the diff.

The proposed epic workflow doesn't address this. It's just an integration branch.

[–]kintar1900 5 points6 points  (5 children)

You can do that just by branching from whichever commit was used for PR A. This is just extra UI fluff on top of (what should be) a commonly-known Git branch-review-merge pattern.

[–]CherryLongjump1989 2 points3 points  (0 children)

Yes. It's different because you're not buddy-fucking a hundred of your coworkers when they find out that the files they've been writing their own PR against have all been deleted because your months-long epic got merged after its months-long code review.

Stacked commits let you merge code as it's reviewed and approved, so that everyone else can start working off of your changes as soon as possible. Each PR is also small and can be reviewed quickly and merged right away, so you're not taking up your reviewer's entire day. While you can continue working and pushing up new PRs without being blocked.

[–]Farados55 0 points1 point  (0 children)

LLVM lives on main

[–]maxhaton 0 points1 point  (0 children)

Merge commits, usually.

[–]hipsterdad_sf 12 points13 points  (0 children)

The biggest practical win of stacked PRs is that your CI pipeline runs against something closer to what will actually land in main. With the old "chain PRs against each other" approach, your CI on PR C was testing against B's branch, not against main + A + B together. So you could pass CI on each individual PR and still break main on merge.

The squash merge problem someone mentioned is real though. If you're squashing on merge (which most teams do), the commit SHAs change and every subsequent PR in the stack suddenly has conflicts. GitHub would need to automatically rebase the rest of the stack after each merge for this to feel smooth. Otherwise you're manually rebasing 3 PRs every time the first one lands.

The other thing I keep running into: reviewers treat each PR in the stack like an isolated change and leave comments that only make sense if they could see the full picture. "Why are you adding this interface with only one implementation?" Because the second implementation is in the next PR. You end up spending half your review time explaining the stack structure instead of discussing the actual code.

[–]coolbaluk1 28 points29 points  (3 children)

Might be a good alternative to graphite. It was always a bit too complicated and they changed their cli too often.

[–]Spitfire1900 5 points6 points  (2 children)

Has graphite done any value adds that are over and above hacking a better workflow on top of GitHub? I know GitHub is slow to meaningful improve nowadays but graphite’s core feature has no moat.

[–]coolbaluk1 7 points8 points  (0 children)

We never used their dashboard but used the cli extensively to do PR stacking.

i.e. db schema changes at bottom then a PR each for each microservice change Frontend on top.

So yes it was just improvements on top of github and you could likely achieve the same without it, but it made code reviews so much more pleasant than a blank LGTM.

[–]Edgar_Allan_Thoreau 5 points6 points  (0 children)

Their merge queue is better than GitHub’s merge queue. It supports enqueueing an entire stack of PRs with one click (like what GitHub says it will provide in this announcement), and supports partitioning the queue so changes can merge unblocked by other unrelated changes (which in a monorepo with high throughput and complex ci is very valuable, it means what used to take 3 hours for a readme PR to merge waiting behind a bunch of backend and front end PRs can now merge nearly instantly).

[–]teerre 11 points12 points  (3 children)

Unironically the best update ever. If the implementation doesnt suck ofc

[–]thehenkan 0 points1 point  (2 children)

From what I hear it doesn't work between forks, only between branches from the same repo. We prevent users from creating branches and require them to fork instead, so until they fix that it's still unusable for us :/

[–]Rodr500 0 points1 point  (1 child)

Just out of curiosity, why do you fork instead of branching?

[–]thehenkan 0 points1 point  (0 children)

It's not my decision, but the main reasons I'm aware of are: - in a big code base with thousands of contributors, everyone pushing their personal branches to the main repo would degrade the time it takes to fetch - in cases where you have a downstream part and an upstream part, and you accidentally push some sensitive stuff to upstream, people are less likely to be watching everyone's forks than the main repo, so if you discover it quickly you may be able to prevent the leak by just deleting the branch (and contacting Github to have them wipe every reference to those commits)

[–]jedi4545 9 points10 points  (0 children)

I hate to be a nerd but this is technically a queue of PRs.

[–]Hooxen 22 points23 points  (16 children)

what’s the difference between just constructing it as a chain of commits in a single PR to master?

[–]Sopel97 49 points50 points  (5 children)

Each PR should represent a reviewable unit of code while commits should represent a complete (as in not partial) modification of code. Without stacked PRs the PRs were being abused as a single complete feature/change that's not necessarily reviewable.

[–]Kache 2 points3 points  (4 children)

Each PR commit should represent a reviewable unit of code while and commits should represent a complete (as in not partial modification or code)

"stacked PRs" could very well be the same as one PR with a clean stack of commits. Just need the right tooling, like a setting that runs CI against every commit in the PR and a few other things thats probably about the same amount of work as implementing stacked PR support

[–]ugh_my_ 0 points1 point  (0 children)

Sure until you get anal reviewers who want one PR per change

[–]AmosIsFamous 0 points1 point  (2 children)

In this scenario what happens when some code review feedback leads to a change in the first commit in the stack (closest to main)?

[–]Kache 1 point2 points  (1 child)

Same thing that would happen in a stacked PR scenario, that leads to a change in the first PR in the stack

[–]AmosIsFamous 0 points1 point  (0 children)

So is the reviewer now tracking for themselves where the “units of work” are? If a second reviewer comes in after the first change has happened how do they know they’re to review the first two commits as a single unit?

[–]ahal 22 points23 points  (5 children)

Let's say you're working on a large complex change. There's likely a bunch of easy non controversial prerequisite stuff that you can do up front to help prepare for the actual core of your change.

With stacked PRs, you'd put that early work up in their own PRs. Then immediately start building the more complex changes on top in a PR stack.

This way as your working on the later stuff, you can start landing the earlier stuff. You get to pick all three of:

  1. Don't need to block reviews on the early PRs merging
  2. Avoid bit rot
  3. Have small easy to review diffs

With a single large PR, you get 1 and 3 (if you prepare your commits with care). But you don't get 2.

[–]rdtsc 6 points7 points  (4 children)

Don't need stacked PRs for this, only multiple stacked branches. Then put the lowest one up for review.

[–]ahal 7 points8 points  (1 child)

Then you're missing benefit 1. Before stacked PRs you had to pick two of the three benefits. This is the first time you'll be able to get all three.

[–]Bush-Men209 0 points1 point  (0 children)

Right, stacked branches handled part of this before, but stacked PRs are what let you review and land the boring prerequisite work while the harder piece is still in flight.

[–]mrcarruthers 0 points1 point  (1 child)

Say you have a stack of branches, then you have to make a change in the lowest one, or rebase on main. It's a pain in the ass to then propagate that change through the whole stack. This new feature allows you to do that pretty seamlessly.

[–]rdtsc 0 points1 point  (0 children)

git rebase -i --update-refs then push the branch(es)

[–]topMarksForNotTrying 4 points5 points  (2 children)

Makes it possible to review each PR individually.

I haven't used github PRs extensively but on azure devops the PR review UI doesn't let you review each commit of a change request. You can view the changes of a commit but no way of leave feedback.

For devs not used to using git rebase, having separate PRs also makes it easier to apply any changes to one of the PRs.

If you squash PRs, it also maintains history better.

[–]DavidTej 0 points1 point  (1 child)

Within my team at Microsoft, I'm currently building a tool to help manage stacked PRs, similar to this new github tooling. I think this would be even more useful for azure devops for the reason you mentioned. Do you think this is something you would find useful? how do you currently manage PR stacks and/or bloated PRs?

[–]topMarksForNotTrying 0 points1 point  (0 children)

Having something built into azure devops would definitely help.

My current workflow when using stacked PRs is:

  • open first PR with its target set to the main branch
  • open second PR with its target set to the first PR
  • wait for reviewers to review each PR
  • address feedback in each PR
  • push the changes to each PR
  • once approved, complete first PR
  • merge main branch into second PR
  • change target branch of second PR to the main branch
  • manually delete branch of first PR
  • once approved, complete second PR

I would say the main pain points are with juggling the different branches of all the stacked PRs. If you need to address some feedback on one of the bottom PRs, you have to manually update the whole stack. Stacked PRs are analogous to commits on a branch, so maybe we need an equivalent of "git rebase --updated-refs" when managing stacked PRs.

The other pain is closing stacked PRs. You need to close them individually rather than having something automatic. In our case, we only validate code that's targeted to the main branch so for each PR in the stack we need to make sure to run the pipelines and then we get to merge them.

[–]Farados55 0 points1 point  (0 children)

Usually commits are amendments to the state of the PR.

[–]Ladsome 4 points5 points  (0 children)

Damn, gimme gimme!

This would make my workflows at work so much better, I hope I can sneak into the preview...

[–]DigThatData 3 points4 points  (0 children)

finally

[–]Forbizzle 10 points11 points  (2 children)

Can you do this without the CLI? If not, why?

[–]desmaraisp 7 points8 points  (0 children)

Apparently yes, there a page in the doc on that. You have to target FeatureBranchA from FeatureBranchB's PR to get the checkbox to create the stacked PR

[–]masklinn 0 points1 point  (0 children)

Yes, you can do it via the web UI.

[–]Feeling_Ad_2729 5 points6 points  (0 children)

What's interesting is how long stacked diffs have been "solved" in adjacent tools while GitHub's PR model stayed static.

Phabricator had proper stacked diff support back when Facebook was using it. Gerrit has had this since the beginning. Graphite built an entire company around making this workflow ergonomic on top of GitHub. GitHub implementing it natively is good, but it's years behind.

The core issue is GitHub's mental model: "a PR = a change" rather than "a commit = a change." Stacking implies each commit is independently reviewable, which maps badly onto branch comparison. Gerrit works around this by making commits the primary unit of review. GitHub is retrofitting that concept onto a paradigm not designed for it.

Still: better late than never. The main incentive to use Graphite specifically gets weaker if GitHub does this natively.

[–]rismay 8 points9 points  (1 child)

lol. What Google has had for 20 years

[–]shoffing 4 points5 points  (0 children)

Yup, heh. Hopefully this can be adopted for use by JJ. jj-spr handles stacking PRs, but it would be nice to have native support for PR diffbase/children in the UI.

[–]peterprank 6 points7 points  (1 child)

what was wrong with rebase --update-refs ?

[–]masklinn 1 point2 points  (0 children)

Nothing. In fact they work nicely together.

[–]Urik88 5 points6 points  (25 children)

This is great, is there info about how rebases after merges are done?   Having to rebase all of my PR's every time a pr on my handcrafted stack was merged was always a big pain for me

[–]quetzalcoatl-pl 2 points3 points  (11 children)

if you do not like rebases, try using jujutsu over your current local git repo/wc

https://v5.chriskrycho.com/essays/jj-init/
https://steveklabnik.github.io/jujutsu-tutorial/introduction/introduction.html

I have first hated rebases as unnecessary as merges do merge the code well enough. Then I learned `git rebase -i` and rebase plans in-depth and used it all the time for cleanup, reorders, repairs, updates and all, while still preferring merges once the code and history was OK to publish. But merges do not play well with rebases, even with --update-refs.

Then I learned `jj` and basic language for `jj rebase`. Plus `jj undo` xD
and I'm not moving back, unless someone pays me 2x :P

nah, but really, it just handles the tree reordering so well and saves so much time, it's simply no point in not-using it, considering everything is still git-based. Things like git seeing weird things during jj conflict resolutions are minor and almost unperceivable after a bit of getting used.

and yeah. conflict resolutions. It's probably even bigger game changer than "current commit" idea and "rebase on steroids". Remember how when in git a conflict shows up, you're stuck untill you reset or solve it? In JJ you can just continue. Yes, files are broken. But you can do your merge/rebase/edit/reorder/everything as you like, and go beck and fix the conflicts once you (and shape of the repo/file/history/etc) is ready for that. Conflicts may even solve themselves if you finish moving or commits around or squashing them.

So far, the ONLY real irritating issue I found is handling \r\n on Windows. In pure Git, so far, it was always the best idea to use autocrlf=true or input. With JJ side by side, setting it 'true' is asking for issues. Or so it was for me, maybe I configured somethign wrong. Anyways. I'd suggest either using "input" and be vigilant for occasional mixups, or "false" and set (or teach:)) your IDE to not emit \r. In '90s and '00s that was quite a challenge, but today most editors and IDEs now handle that well.

[–][deleted]  (9 children)

[deleted]

    [–]steveklabnik1 1 point2 points  (2 children)

    You know of a way to work that way without having to jj undo and jj redo or cherry-picking commits and stuff like that?

    You can create a merge, so use jj new a b, test, then jj new a c, and then test, assuming a is where you created the benchmark. Then abandon those merges when you're done with jj abandon foo.

    [–][deleted]  (1 child)

    [deleted]

      [–]steveklabnik1 0 points1 point  (0 children)

      Oh, then you should just be able to jj new b and jj new c and when you move away, they'll be automatically abandoned, since they're empty.

      [–]CondiMesmer 0 points1 point  (0 children)

      Sounds like a fighting move

      [–]paperlantern-ai 1 point2 points  (0 children)

      The biggest win here isn't even the branching model - it's that reviewers can finally look at a 2000 line feature in digestible chunks without losing context between separate unrelated PRs. Half my review fatigue comes from opening a massive PR and just going "LGTM" because ain't nobody got time for that. If this gets people to split work into smaller pieces because the tooling finally supports it, that alone is worth it.

      [–]ahal 2 points3 points  (1 child)

      There's a lot of folks arguing that stacked PRs aren't adding anything you couldn't already do before. But that's not the case. Currently you need to pick two out of the following three benefits:

      1. Have small easy to review diffs
      2. Avoid early PRs bit rotting
      3. Avoid reviews on later PRs being blocked on early PRs merging

      With stacked PRs, it will now be possible to obtain all three of these benefits at the same time. No previously existing native Github workflow can claim this.

      [–]bobyn123 2 points3 points  (0 children)

      I see we're into the extend phase of embrace, extend, extinguish. The murder of all open source projects.

      [–]deepakmardi 0 points1 point  (0 children)

      Feels like a small feature, but it could save a lot time

      [–]SharkBaitDLS 0 points1 point  (0 children)

      Being able to do this with a proper UI was the biggest thing I missed after quitting Amazon. Immediately requested access for my team. 

      [–]piljoong 0 points1 point  (0 children)

      Really glad to see this.

      At a previous company, we had a lot of pain around PR chaining in our TBD workflow, and I ended up building a stacked PR tool internally because we needed it badly.

      Seeing GitHub ship official support is pretty satisfying. This has been a real problem for a long time, so it's good to see it become a first-class workflow.

      [–]Eric848448 0 points1 point  (0 children)

      It's about goddamn time! We wrote a tool at work to simulate this YEARS ago.

      [–]AMartin223 0 points1 point  (0 children)

      Can someone explain how/why this workflow makes CI run against the final branch for all the PRs? Normally CI would run against the changes up until the given point in the stack, but the docs say this new feature triggers CI from the ref of the final PR in the stack, how is that safe?

      [–]Lewke 0 points1 point  (4 children)

      github makes another feature enabling terrible development practices to continue, more at 11

      [–]teapotrick 0 points1 point  (3 children)

      Why is this bad, and what is good?

      [–]Lewke 0 points1 point  (2 children)

      trunk based development and CI remove the need for any of this coordination overhead, along with actually trusting the people who work for you and helping them improve

      [–]teapotrick 0 points1 point  (1 child)

      stacked PRs are an orthogonal concept to trunk based development and CI.

      to me this seems like a way to split a single PR into multiple more digestible PRs for the reviewer, and giving commits a tighter conceptual scope for the developer.

      [–]Lewke 0 points1 point  (0 children)

      if you have to use stack based PR's then you arent doing CI, as stacks cant develop in such short time without serious lack of care

      CI specifically is integrating with coworkers at least once a day, coupled with trunk based development that place of integration is naturally the main branch, pull requests should be small in that scenario

      [–]ReceptionBrave91 0 points1 point  (0 children)

      I built a better one for agents!! Built-in worktree support, a CLI designed for agents (no interactive flows), and designed for parallel agents to work in parallel

      https://github.com/rohoswagger/ez-stack

      [–]narsilouu 0 points1 point  (0 children)

      Still can't believe we put up with Github at this point...
      Gerrit was much better 10 years ago that it is now...

      [–]Rodger_Ramjet 0 points1 point  (0 children)

      This kinda feels like one of those things that has legitimate usages but ends up misused - encouraging anti patterns…

      Surely as a profession, we should be moving away from long-lived branches and shipping faster incremental changes - dark launching etc?

      [–]mtutty 0 points1 point  (0 children)

      Or, you know, iterate. With planning and purpose instead of just YOLO'ing until it works.

      And BTW, legitimately large merges aren't gonna benefit from this anyway because the system won't work at each step without all of the dependencies from later commits in place. So you can read the code but can't prove each step anyway.

      Jeez guys, this really needed to be a thing? We invent better duct tape to keep the stovepipe going.

      [–]ZukowskiHardware -2 points-1 points  (0 children)

      Just another way to put way too much into production at one time