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

181–190 of 308 posts

Re: Stacked PRs are now live on GitHub

#181

Post by ghthor »

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

Earlier quoted context omitted.

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

Indeed. But lots of people have never even heard of Gerrit :P. And the implementation feels very much like it sits in the UI layer, rather than further down the stack. I'm not trying to claim stacking PRs in this way is the best way (it's not). But it does add an extra affordance for those who want it without burdening those who don't with the need to understand why someone would prefer it. And there are also plenty…

I’m a pretty clean committee. About every graphite stack I’ve worked on for the past 2years has been single commit per PR. The tooling managing all the rebasing and PR bases is great.

Re: Stacked PRs are now live on GitHub

#182

Post by ghthor »

mrkeen wrote: post #83
Insimwytim wrote: post #23

I feel like many people (and industry in general) complicate things unnecessary. Stacked pull requests break large changes into small, reviewable pull requests. That's how pull requests are supposed to be, no? If yours aren't that - you ought to rewrite them. With stacks, you can independently review and check each pull request, then merge everything together in one click. Why would I want to do that instead merging…

I don't mind rebasing, so that's not really an issue. The problem I sometimes have is when I queue up 4 or 5 PRs in the afternoon and expect someone to take a look in the morning. The diff of A into master looks OK, but then the diff of B looks like AB, and the diff of C looks like ABC - each PR has changes that will have already been merged once earlier PRs have been accepted. My way around it was to raise E as a PR…

This is stacking but with getting no help from the platform tooling or UI. Your ready and stacking feels great. I’m a 2 year user of graphite and it really makes work a lot more fun not managing all the PR, bases, etc. make good commits, split each into a branch:PR, record faster, async progress while getting reviews, etc. it’s good stuff

Re: Stacked PRs are now live on GitHub

#183

Post by nirvdrum »

necovek wrote: post #173

I dislike them reinforcing the component approach to delivering work through their examples, like the top screenshot showing "database schema changes", "api changes" and "frontend implementation" as separate branches in a stack. So really, one does consider full stack a single feature, but unless they are reviewed in one go — which defeats the purpose of stacked branches and pull requests — you can end up landing one…

Thanks for mentioning this. It seemed odd to me, too, so I spent some time trying to work it out. As a reviewer, I'm not sure how I'm supposed to assess database or API changes without knowing how they're intended to be used. And deploying them independently seems odd, too, especially if you need to roll it all back.

I think in my ideal world there would be a clean history and I could review a PR commit-by-commit. But, you can't just approve a single commit, so there's a tooling problem there. And most CI runs on an entire push rather than individual commits. And increasingly I see devs using git as an offsite backup for whatever change they just made, rather than breaking commits up into logical chunks. In that workflow, squashed merges make the most sense.

It's probably flawed, but the mental model I came up with is each stacked PR collapses into what would have been an individual commit in a clean PR, with the advantage of being able to be reviewed separately from the other changes and forced to clear CI. And then the whole stack becomes what would have been a clean PR in the old model. That, I can kinda see the benefit of. But, merging only part of the stack into trunk is the mental hurdle I can't clear; it'd be like merging only some commits from a PR. It kinda reminds me of when projects used CVS.

I've really only seen stacked PRs used on projects where history is little more than an audit log. I'm keen to see how this gets employed by open source projects. I think there's a disconnect and it's likely I'm not going to really get it until I see it.

Re: Stacked PRs are now live on GitHub

#184

Post by whichdan »

The issue I have with stacked PRs isn't merging, but rather, when I change branch A, and I'm working on branch C, I then need to merge A into B and then B into C just to continue working. Collapsing/merging them at the end is the easy part.

Re: Stacked PRs are now live on GitHub

#185

Post by ollysb »

nirvdrum wrote: post #183
necovek wrote: post #173

I dislike them reinforcing the component approach to delivering work through their examples, like the top screenshot showing "database schema changes", "api changes" and "frontend implementation" as separate branches in a stack. So really, one does consider full stack a single feature, but unless they are reviewed in one go — which defeats the purpose of stacked branches and pull requests — you can end up landing one…

Thanks for mentioning this. It seemed odd to me, too, so I spent some time trying to work it out. As a reviewer, I'm not sure how I'm supposed to assess database or API changes without knowing how they're intended to be used. And deploying them independently seems odd, too, especially if you need to roll it all back. I think in my ideal world there would be a clean history and I could review a PR commit-by-commit. Bu…

This is my confusion, we already have commits to bundle changes, why not simply allow commits to be reviewed independently within a pr?

Re: Stacked PRs are now live on GitHub

#186

Post by crakhamster01 »

sunshowers wrote: post #149
sameenkarim wrote: post #44

Hey from the GitHub Stacked PRs team! Excited to release this more broadly so anyone can start stacking: https://gh.io/stacks Would love to hear any feedback, especially with the UI and CLI. We've got a lot more updates to the PR experience in store! Also happy to answer questions about the design decisions we made. There's a bunch happening behind the scenes, and it's one of the largest launches in GitHub history co…

I apologize for being somewhat direct but what prior art did you engage with? Why are you making people create a branch for each change in a stack? Why do developers have to create new commits when iterating on the PR? Where is the proper support for interdiffs? What about change IDs? The fundamental issue with GitHub -- really, its original sin -- is that the review model is wrong. It encourages a new commit + merge…

Having also worked with the tools you mentioned, is using commits in-place of "versions" + squash merging all that different?

  1. Adding commits means reviewers can easily diff changes to the PR
  2. Squash merging preserves only 1 commit per PR ends up on trunk

Re: Stacked PRs are now live on GitHub

#187

Post by aizk »

sameenkarim wrote: post #52
lucky_cloud wrote: post #5

Is that why the menu toggle is the stack of pancakes emoji (U+1F95E)? Whimsy is fine but that change made me super suspicious about what I was looking at.

We've been using the pancake emoji internally so we thought it was a fun easter egg. It'll only be up for a few hours and then will go back to the regular icons :)

You should keep it for a bit longer

Re: Stacked PRs are now live on GitHub

#189

Post by CBLT »

miovoid wrote: post #129
sameenkarim wrote: post #44

Hey from the GitHub Stacked PRs team! Excited to release this more broadly so anyone can start stacking: https://gh.io/stacks Would love to hear any feedback, especially with the UI and CLI. We've got a lot more updates to the PR experience in store! Also happy to answer questions about the design decisions we made. There's a bunch happening behind the scenes, and it's one of the largest launches in GitHub history co…

Why not to make it part of Git project? Such fragmentation adds complexity and vendor locking.

I'm trying to understand what you mean, because it's not obvious to me at all. If you read LKML you'll see stacked PRs has been the norm for years. I'm assuming you want the `gh stack` porcelain into the git cli? We'd first need to add PRs as a porcelain to the git cli for it to make any sense, right?

Re: Stacked PRs are now live on GitHub

#190

Post by masklinn »

mattmatheson wrote: post #144
Game_Ender wrote: post #111

Earlier quoted context omitted.

Can you make sure there is good API support for stacks? We use a custom merge queue and we want it to be able to land multiple PRs from a stack at once as separate PRs. Last I checked you had to land a single PR, rebase the stack, land the next and so on. This is very expensive in CI time (and wall clock time), vs simply testing part or all of a stack in parallel then declaring those merged. In essence a robot needs…

Hey there - we worked with Sameen and the team at GitHub over the past month to get support for stacks in our Mergequeue: https://trunk.io/blog/trunk-merge-queue-now-supports-github-... They do indeed have APIs you can use in your mergequeue, I'm happy to share notes on how we built it so you can add it to your mergequeue.

Does trunk use GitHub’s api to merge stacks, or does it do the integration internally / on its own?
Post reply on HN