Hacker News Board Hacker News, browsed like a classic message board
Live data from Hacker News

Stacked PRs are now live on GitHub

github.blog

Post reply on HN
Print view View original

131–140 of 308 posts

Re: Stacked PRs are now live on GitHub

#131

Post by piskov »

rrradical wrote: post #18

I thought 'stacking' PRs was useful in two circumstances. 1. The PRs are across different related repos, so they literally can't be combined into one PR. 2. You want to keep producing work while the first PR is in review. So you stack subsequent PRs onto the same branch. Basically just pipelining. But this feature doesn't seem to hit either use case, and instead just seems to be a different form of stacking commits i…

> 2. You want to keep producing work while the first PR is in review. So you stack subsequent PRs onto the same branch.

Instead of using the same branch, make new branches from that parent and commit there.

The cool thing about that approach is that (at least in git-tower app) is when you edit a parent branch after pr comments, all those new commits will be automatically “restacked” on descended branch (children branches will be rebased on new state or parent, incorporating the hew fixes)

Re: Stacked PRs are now live on GitHub

#132

Post by chillfox »

teeray wrote: post #43
Okkef wrote: post #17

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 w…

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…

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.

Re: Stacked PRs are now live on GitHub

#133

Post by ahepp »

Okkef wrote: post #17

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 w…

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.

Re: Stacked PRs are now live on GitHub

#135

Post by mosura »

an0malous wrote: post #113
matharmin wrote: post #13

I've been using the preview for a bit, and I'm quite surprised to see them expanding the preview with so many unfixed issue. For example, merging an entire stack is completely broken in many cases: https://github.com/github/gh-stack/discussions/212 You can merge one by one, but if you're using squash and merge, you need a re-approval for each PR in the stack if you require reviews. This makes you lose out on arguably…

the whole industry has become completely “ready, fire, aim” since 2021

GitHub has been especially bad.

Ultimately they have become a monopoly so it is kind of expected.

Re: Stacked PRs are now live on GitHub

#136

Post by ahepp »

paxys wrote: post #77
Okkef wrote: post #17

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 w…

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.

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.

Re: Stacked PRs are now live on GitHub

#137

Post by lnrd »

Okkef wrote: post #17

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 w…

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

Re: Stacked PRs are now live on GitHub

#138

Post by saghm »

matharmin wrote: post #13

I've been using the preview for a bit, and I'm quite surprised to see them expanding the preview with so many unfixed issue. For example, merging an entire stack is completely broken in many cases: https://github.com/github/gh-stack/discussions/212 You can merge one by one, but if you're using squash and merge, you need a re-approval for each PR in the stack if you require reviews. This makes you lose out on arguably…

> For example, merging an entire stack is completely broken in many cases: https://github.com/github/gh-stack/discussions/212

> You can merge one by one, but if you're using squash and merge, you need a re-approval for each PR in the stack if you require reviews. This makes you lose out on arguably the biggest gain of stacked PRs.

I'm struggling to imagine what it offers at all if that doesn't work! Is it just a way of manually marking another MR as a dependency in the UI so that it shows up with a red X if the other one isn't merged yet?

Re: Stacked PRs are now live on GitHub

#139

Post by mcintyre1994 »

ahepp wrote: post #136
paxys wrote: post #77

Earlier quoted context omitted.

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.

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.

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.

Re: Stacked PRs are now live on GitHub

#140

Post by atq2119 »

4lx87 wrote: post #79
andrewaylett wrote: post #58

Earlier quoted context omitted.

They don't. But reviewing individual commits in the GitHub UI is hard. A set of stacked PRs is exactly the same as a line of commits. The only difference is the UI, but the UI is the important bit here because lack of UI is what's stopping folk from doing that today. Even when I've developed my changes as a stack of commits, I'll feed them to my team one commit (and one PR) at a time so they're easier to review — and…

> They don't. But reviewing individual commits in the GitHub UI is hard. So instead of solving that problem, GitHub developed tooling around a workaround for that problem (targeting a PR at another branch that also has a PR). Expanding reviews to allow per-commit reviews avoids the need for managing additional branches and all the headache that comes with it.

The sad fact of life is that too many developers are used to pushing PRs with unprincipled commits, and relying on squash merges.

The approach that GitHub chose can show those people almost immediate benefits, which is surely the better path towards adoption than trying to reeducate everyone to adopt a development flow based on clean rebases.

And for those of us who do prefer clean rebases, the thing they built is still useful.

A bunch of details aside, the major conceptual thing that the email based flow has which is still missing in GitHub's data model is the ability to have a discussion on the stack as a whole.

Post reply on HN