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

221–230 of 308 posts

Re: Stacked PRs are now live on GitHub

#221

Post by fphilipe »

MBCook wrote: post #176
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 tried to look through this earlier. Am I right this is only available through the CLI? If so it’s a no go for me and my team. Which is too bad because it looks quite useful. An addition ability I would love, which is a MUCH bigger feature and I recognize that, would be multi-repo stacks. My company doesn’t use a monorepo, and I like that. But as we’ve been breaking monoliths sometimes a logical feature touches mult…

I was using my usual stacked PR approach without the CLI and it detected the stack. So I think the CLI just does the heavy lifting for you (although I prefer to stick to vanilla git rather than learning a new tool).

Re: Stacked PRs are now live on GitHub

#222

Post by eddythompson80 »

beaker52 wrote: post #220

Looking at the way GitHub are selling this “feature”, I feel like some of the engineers who are going to be excited about this feature for “reviewability” reasons are, in particular, those who’ve forgotten that they should be splitting changes into multiple logical commits inside a PR. And instead of that they’re now going to use multiple, single commit branches and stack them because stacked PRs are a “new” “feature…

I think plenty of people explained the difference between stacked PRs and individual commits in 1 PR. You’re not wrong, but it’s just a matter of path of least resistance. There is no way to leave a comment on a particular commit in a PR. Also all commits addressing PR feedback get tucked at the bottom of the commit list. Unless you do some crazy git gymnastics and rewrite the PR history and confuse everyone. With stacked PRs you also (as a maintainer or a reviewer) have the option to merge some and not others. Like here are 3 stacked PRs, one for provisioning some AWS or Azure resources that I’ll need, one for implementing the APIs using those resources, and one for updating the UI to use the new APIs. You can then say “let’s get the 2 backend PRs in and hold off on the UI as we’re changing that entire view”. You can’t do that with multiple commits in a PR without asking the person to redo the PR, then you’re back to git-foo.

Yes, GitHub could have made the UI allow a “per commit” comments somehow, then allow you to select the set of commits to include in the merge somehow, then write a blog post on how to manage “Address PR comments #1” commits. But the stacked PRs solve all that. Not to mention how people treat commits as their own internal save states. I always enable “squash and merge” option because I think it makes a lot more sense to have 1 commit on main per PR where all the context of the change is either in the commit message or the linked PR.

Also LLM providers charge per token. Charging per “work unit” is still not a solved problem. You can’t charge per “review” when your cost is per token. Just like airlines can’t charge “per ticket”, they have to charge differently depending on the destination. Unless you invent some bs arbitrage to lure users and eventually bait and switch on them.

Re: Stacked PRs are now live on GitHub

#223

Post by flexagoon »

ollysb wrote: post #185
nirvdrum wrote: post #183

Earlier quoted context omitted.

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?

This is similar to how things like Gerrit do code review, and it's pretty nice

Re: Stacked PRs are now live on GitHub

#225

Post by sarthak-ag »

sepeth wrote: post #11

One of the nice things about jujutsu related to this is that when you update a branch, it rebases other branches started off of that branch. I often switch to jj if I want to split my work for easy reviewing, and it works great colocated with a clone created with git.

Yes, and in fact, JJ is much more than that. One of the other features I use quite heavily is that, in JJ, you can essentially check out multiple branches at the same time by just creating a local merge. JJ will keep your local merge updated as you rebase the branches it builds on. I have written about it here: https://sarthakag.bearblog.dev/from-git-to-jj-jujutsu/

Re: Stacked PRs are now live on GitHub

#226

Post by tao_oat »

hypendev wrote: post #218
tao_oat wrote: post #26

Earlier quoted context omitted.

Here's a common flow where I find stacked PRs are useful: - I want to build feature X - Ah, but it would work better if I refactored the module first - I refactor then build feature X - There's then some additional (and optional) cleanup work As a reviewer I wouldn't want to see all this in a single PR, and the changes depend on each other so I can't open multiple independent PRs. Manual rebasing is fine but navigati…

Why isnt refactor in its separate PR? Every developer with an ounce of thought would tell youn to: >Do PR refactoring module A >Do PR with feature X + cleanup >Maybe PR with additional work if bigger

I think we're saying the same thing, that's how I use stacked PRs.

Re: Stacked PRs are now live on GitHub

#227

Post by ivolimmen »

I personally see no benefit using this. For large changes I often completely work locally until it's done. I commit everything logically and when everything is done I push and make one PR. The commits are not fully done as they work towards the main goal of the PR. If the team argues before the start of the feature that the individual commits are needed we split the ticket. This stacking sounds a lot like shit you need for shit made by LLM's

Re: Stacked PRs are now live on GitHub

#228

Post by eddythompson80 »

ollysb wrote: post #185
nirvdrum wrote: post #183

Earlier quoted context omitted.

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?

Ok, then how do you approve/take the first 2 commits and not the last one? Or how do you insert a commit in between 2 commits or address feedback on a given commit?

The person making the change is now going to have to run multiple confusing interactive rebases and git shenanigans, the rewrite the history of the PR branch on every feedback, then you have to re-review all the commits again because they are all different.

It’s possible of course to push all that complexity on the tooling. Have GitHub and git provide tooling for doing all that within the context of a single branch/PR. But why is that better? Multiple branches are easier to manage in git, and as long as they don’t conflict on the merge. Obviously if a feedback on PR#1 causes a conflict in PR#2 which causes a conflict in PR#3 it’s still tedious, but it’s a lot more doable than managing interactive rebases on every feedback comment.

Re: Stacked PRs are now live on GitHub

#229

Post by eddythompson80 »

ivolimmen wrote: post #227

I personally see no benefit using this. For large changes I often completely work locally until it's done. I commit everything logically and when everything is done I push and make one PR. The commits are not fully done as they work towards the main goal of the PR. If the team argues before the start of the feature that the individual commits are needed we split the ticket. This stacking sounds a lot like shit you ne…

The stacked PR pattern long predates LLMs. Yes LLMs make them more common and frequently steer you in that direction. But it’s by no means a new “LLM” thing.

In the scenario you described of your team splitting the tickets, this gives you a UI to manage the PRs of the multiple tickets so you are no longer blocked on merging PR#1 before you can proceed to PR#2. Yes you might have to deal with conflicts, but you would have had to deal with that regardless.

Re: Stacked PRs are now live on GitHub

#230

Post by eddythompson80 »

hypendev wrote: post #217
dastbe wrote: post #21

Earlier quoted context omitted.

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

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?

> 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

Post reply on HN