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

51–60 of 308 posts

Re: Stacked PRs are now live on GitHub

#51

Post by sameenkarim »

lucideer wrote: post #27

Started using the gh stack CLI when I first heard of this feature & really liked it - great tooling. Then got approved for the preview & found the corresponding web UI features incredibly underwhelming. Pre-approval the CLI tooling effectively enables easier automations around splitting a task into multiple atomic PRs - really great locally but then when you push they just show up as independent unlinked PRs. Post-ap…

I hear you. We had to start with something a bit more minimal, but we are working on a much broader revamp for the PR UI. Part of that will include a persistent view of the stack so you always know you're working with a stack and easily navigate between the layers without a ton of clicks.

Re: Stacked PRs are now live on GitHub

#52

Post by sameenkarim »

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

Re: Stacked PRs are now live on GitHub

#53

Post by satvikpendem »

mmlb wrote: post #35
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've had that same "what's the point" thought every time I've read about stacked PRs, but recently had an (obvious) epiphany. The benefit is you get CI for each commit! I've always hated fixup/typo/fix tests commits and toyed with having a CI check that enforced ci passing on each commit but this drops that need.

I don't get it, we already have CI for each commit, at least at our workplace, enforced.

Re: Stacked PRs are now live on GitHub

#54

Post by steveklabnik »

This is one of the biggest changes to hit GitHub in many years. I'm really glad to see something like this deployed to one of the largest forges in the world, hopefully it will expose a lot of developers to workflows that they didn't even know about before.

If you buy the idea that stacking produces better software, then this also has the opportunity to really help out quite a few people.

Re: Stacked PRs are now live on GitHub

#55

Post by selimthegrim »

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…

what if you stack a draft PR on top of a regular one?

Re: Stacked PRs are now live on GitHub

#56

Post by dualvariable »

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?

What I can see is that you can easily append commits to e.g. the first PR in a stack, which would insert them into the middle of sequence of commits.

This will require rebasing and fixing the subsequent PRs in a stack the same way you'd need to rebase and fix the subsequent commits in a mega-PR. But it makes the right thing easy (keeping all the commits to the foundation of the change together) rather than makin the wrong thing easy (appending fixup commits across the entire change in a random order so that the actual foundational change is lost).

Keeping all the foundational commits together also keeps all the discussion over the foundational change together.

You could argue that you'd want to only do the foundational PR and stop, but doing the whole stack of PRs gives the reviewers more information about where you're going, and allows work to continue asynchronously.

Re: Stacked PRs are now live on GitHub

#57

Post by theappsecguy »

literallyroy wrote: post #45
theappsecguy wrote: post #24

Earlier quoted context omitted.

I'd recommend git-spice, it's very easy to use and powerful, and of course open-source. I've tried graphite but found that they made it too convoluted for what it is.

How does git-spice compare to git-town?

I haven't used git-town, but from a cursory look it appears to be a various collection of gitops improvements for day to day things, including some for stacking.

git-spice is specifically targeted to be useful for PR stacking and doesn't require you to do anything differently from normal git operations that you likely use already. It has a bunch of really nice flows and doesn't try to step outside the bounds of what is needed to easily stack PRs

Re: Stacked PRs are now live on GitHub

#58

Post by andrewaylett »

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 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 I discovered that GitHub had turned on stacked commits UI because for one particular project I'd manually created a set of PRs in advance (with the right bases) and GitHub offered to create a stack out of them.

Re: Stacked PRs are now live on GitHub

#59

Post by skydhash »

steveklabnik wrote: post #47
skydhash wrote: post #42

Earlier quoted context omitted.

> I've always hated fixup/typo/fix tests commits and toyed with having a CI check that enforced ci passing on each commit but this drops that need. Can't you run your CI locally? I know it's not feasible for some codebase, but at least the linting, formatting, unit tests, some integration tests should be able to be done locally.

You can't have mac/windows/linux/whatever all locally simultanously. Not every project requires this, but for those that do, it's impossible. Also, it is much harder to enforce "everyone must run each commit through the CI equivalent properly" than it is when it's on your forge.

> You can't have mac/windows/linux/whatever all locally simultanously.

Why can't you? That's what VMs are for. And even then, most cross-platform codebases have an abstraction layer that rarely changes. So even testing on one platform can raise your confidence very high.

> Also, it is much harder to enforce "everyone must run each commit through the CI equivalent properly"

Again why? I wouldn't care about the dev's local branch. But what is send to the main repo can be easily scripted to run the CI on every commit. You just send the result back with each commit that fails. They can replicate the same workflow on their local workspace as a pre-push process.

Post reply on HN