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

GitHub Stacked PRs

github.github.com

Post reply on HN
Print view View original

231–240 of 548 posts

Re: GitHub Stacked PRs

#231

Post by jiveturkey »

akersten wrote: post #79

Does it fix the current UX issue with Squash & Merge? Right now I manually do "stacked PRs" like this: main If PR B merges first, PR A can merge to main no problems. If PR A merges to main first, fixing PR B is a nightmare. The GitHub UI automatically changes the "target" branch of the PR to main, but instantly conflicts spawn from nowhere. Try to rebase it and you're going to be manually looking at every non-conflic…

You "just" need to know the original merge-base of PR B to fix this. github support is not really required for that. To me that's the least valuable part of support for stacked PRs since that is already doable yourself.

The github UI may change the target to main but your local working branch doesn't, and that's where you `rebase --onto` to fix it, before push to origin.

It's appropriate for github to automatically change the target branch, because you want the diff in the ui to be representative. IIRC gitlab does a much better job of this but this is already achievable.

What is actually useful with natively supported stacks is if you can land the entire stack together and only do 1 CI/actions run. I didn't read the announcement to see if it does that. You typically can't do that even if you merge PR B,C,D first because each merge would normally trigger CI.

EDIT: i see from another comment (apparently from a github person) that the feature does in fact let you land the entire stack and only needs 1 CI run. wunderbar!

Re: GitHub Stacked PRs

#232

Post by monster_truck »

p-e-w wrote: post #224
sunshowers wrote: post #109

Earlier quoted context omitted.

To my knowledge, stacked diffs were first done in the Linux kernel as stacks of patches sent over email. From there they spread to Google and Facebook. (Source: I worked on Facebook's source control team from 2012-2018 and did a lot of work to enable stacked diffs there.)

At some point, a derivative idea becomes so different from the original one that it’s a novel idea in essence. Just like SMS is ultimately a derivative of cuneiform tablets, and yet it isn’t in any meaningful sense.

Imagine gettting a cuneiform tablet by courier telling you that you have unpaid parking tickets in a state you've never driven in

Re: GitHub Stacked PRs

#233

Post by steveklabnik »

dathanb82 wrote: post #226
nine_k wrote: post #95

Earlier quoted context omitted.

Can these tools e.g. do per-commit review? I mean, it's not the UI what's the problem (though it's not ideal), it's the whole idea of commenting the entire PR at once, partly ignoring the fact that the code in it changes with more commits pushed. Phabricator and even Gerrit are significantly nicer.

Unless you have a “every commit must build” rule, why would you review commits independently? The entire PR is the change set - what’s problematic about reviewing it as such?

In stacked diffs system, each commit is expected to land cleanly, yes.

Re: GitHub Stacked PRs

#234

Post by steveklabnik »

altano wrote: post #229

The `gh stack` CLI sounds essential for people using git, but I hope it doesn't become required, as people using things like jj/sl should be able to work with stacks. `gs submit`/`gs push` being the interface is fine, but `gs init` and `gs add` should be optional.

They confirmed below that you should be able to use this with jj just fine, just like you can already use gh to create a PR that you've authored with jj: https://news.ycombinator.com/item?id=47759426

Re: GitHub Stacked PRs

#235

Post by ahmadyan »

jenadine wrote: post #66

I might be missing something, but what I need is not "stacked PR" but a proper UI and interface to manage single commit: - merge some commits independently when partial work is ready. - mark some commit as reviewed. - UI to do interactive rebase and and squash and edit individual commits. (I can do that well from the command line, but not when using the GitHub interface, and somehow not everyone from my team is famil…

the best implementation i've worked with was SuperSmartLog (SSL) at Meta, which was open-sourced at interactive smartlog (https://sapling-scm.com/docs/addons/isl/). There are also extension for it in VSCode, etc.

Surprisingly it never gained the adoption it deserved.

Re: GitHub Stacked PRs

#236

Post by steveklabnik »

normie3000 wrote: post #225
stephbook wrote: post #62

Earlier quoted context omitted.

Commits are immutable and you never know which feedback goes stale when you add another commit. I'm not a huge fan, since stacked PRs mean the underlying issues don't get addressed (reviews clearly taking too long, too much content in there), but it seems they want something that works for their customers, right now, as they work in real life.

> Commits are immutable I guess this is why you're getting downvoted. Commits can be edited.

When you edit a commit, it creates a new commit. They are immutable. You can still find the old commit via the reflog, until it gets eventually gc'd.

If I had to guess a reason they were downvoted (and I didn't downvote, to be clear), it's probably because people see stacked diffs as specifically solving "reviews clearly taking too long, too much content in there", and so it feels contradictory. Then again, as I said, I didn't downvote!

Re: GitHub Stacked PRs

#237

Post by jiveturkey »

WhyNotHugo wrote: post #124

I really don't get the point of stacked PRs. Just using git, you'd send a set of patches, which can be reviewed, tested and applied individually. The PR workflow makes a patch series an undivisible set of changes, which must be reviewed, tested and applied in unison. And stacked PRs tries to work around this issue, but the issue is how PRs are implemented in the first place. What you really want is the ability to rev…

Right, a PR is "just" a set of commits (all must be in the same branch) that are intended to land atomically.

Stacked PRs are not breaking up a set of commits into divisible units. Like you said, you can already do that yourself. They let you continue to work off of a PR as your new base. This lets you continue to iterate asynchronously to a review of the earlier PRs, and build on top of them.

You often, very often, need to stage your work into reviewer-consumable units. Those units are the stack.

Re: GitHub Stacked PRs

#238

Post by mh2266 »

eptcyka wrote: post #100
tcoff91 wrote: post #75

Earlier quoted context omitted.

It's basically trying to bring the stacked diff workflow pioneered by Phabricator to GitHub. The idea is that it allows you to better handle working on top of stuff that's not merged yet, and makes it easier for reviewers to review pieces of a larger stack of work independently. It's really useful in larger corporate environments. I've used stacked PRs when doing things like upgrading react-native in a monorepo. It r…

What if main/master moves in between reviews?

you just rebase it? what's the big deal?

I don't use Github but I do work at one of the companies that popularized this workflows and it is extremely not a big deal. Pull, rebase, resolve conflicts if necessary, resubmit.

Re: GitHub Stacked PRs

#240

Post by sunshowers »

p-e-w wrote: post #224
sunshowers wrote: post #109

Earlier quoted context omitted.

To my knowledge, stacked diffs were first done in the Linux kernel as stacks of patches sent over email. From there they spread to Google and Facebook. (Source: I worked on Facebook's source control team from 2012-2018 and did a lot of work to enable stacked diffs there.)

At some point, a derivative idea becomes so different from the original one that it’s a novel idea in essence. Just like SMS is ultimately a derivative of cuneiform tablets, and yet it isn’t in any meaningful sense.

I don't think mailing stacks of patches is that different? As someone who built this stuff it was pretty obvious to me that web-based patch stack management was a relatively small evolution over mailing lists. Tools like patchwork bridged the gap initially, and we were quite familiar with them.
Post reply on HN