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

71–80 of 308 posts

Re: Stacked PRs are now live on GitHub

#71

Post by esafak »

mchristen wrote: post #32
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…

Those could just be individual commits on a branch. Why does it need to be a stacked PR? That concept really only exists within the interfaces of these kinds of tools.

They could but they don't belong together. A PR should be about one thing. Refactoring one module for another's sake should be done in another PR. If not for reviewing, for revertability, and future debuggability.

Re: Stacked PRs are now live on GitHub

#72

Post by cebert »

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 :)

Good. This change was unexpected and so unprofessional I though my browser was compromised.

Re: Stacked PRs are now live on GitHub

#77

Post by paxys »

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.

Re: Stacked PRs are now live on GitHub

#78

Post by ransom1538 »

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…

If you thought git was complex, wait until you get your 5 deep stacked PR from that nice coworker! Jokes aside this is really an anti-feature. Now coworkers can blow off your PR for much longer.

Re: Stacked PRs are now live on GitHub

#79

Post by 4lx87 »

andrewaylett wrote: post #58
mchristen wrote: post #32

Earlier quoted context omitted.

Those could just be individual commits on a branch. Why does it need to be a stacked PR? That concept really only exists within the interfaces of these kinds of tools.

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.

Re: Stacked PRs are now live on GitHub

#80

Post by masklinn »

kazinator wrote: post #39
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 also don't get it. > With stacks, you can independently review and check each pull request, then merge everything together in one click. Consider: "With pull requests, you can independently review and check each commit inside the pull request, and then merge the entire pull request in one click." Pull requests are stacked commits. This does not have to recurse; you don't need stacked pull requests, not to mention s…

> With pull requests, you can independently review and check each commit inside the pull request, and then merge the entire pull request in one click

Except you can’t really do that on GitHub, the “unit if review” is the PR so reviewing commits is adhoc, inconsistent, and awkward, and tracking their changes as they get fixed up is a pain. “Splatting” that as PRs is not the nicest way to do it green field, but it’s an evolution that makes sense in GitHub’s model.

Post reply on HN