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

In Praise of Stacked PRs

benjamincongdon.me

Post reply on HN
Print view View original

101–110 of 230 posts

Re: In Praise of Stacked PRs

#101

Post by arxanas »

doktorhladnjak wrote: post #6

We use GitHub Enterprise where I work now. I do sometimes do stacked PRs but GHE does not make it easy. The rebasing and merge conflict resolution can be a headache although git rebase —onto helps a lot. I previously worked somewhere that used Phabricator. Its “stacked diffs” worked great. I’d use it all the time when working on complex, multipart changes.

Try out the linked tool git-branchless (https://github.com/arxanas/git-branchless, I'm the author). It should help you restore Phabricator-like workflows. In particular, check out the `git sync`, `git restack`, and `git move` commands for handling rebasing and conflict resolution.

Re: In Praise of Stacked PRs

#102

Post by wizofaus »

nightpool wrote: post #74
chrisseaton wrote: post #56

Earlier quoted context omitted.

> The fact that you have to stack in the first place typically suggests that PRs aren't being merged fast enough. Unless PRs are merged instantly, I'm always going to be waiting after one PR is opened, before I can work on the next, unless I stack, aren't I? Is your definition of 'fast enough' instantly? If not, how does this work?

This is why I think it's really really important that all PR reviews be synchronous , so that there's never any time spent twiddling your thumbs or context switching onto another change. Also it just makes it much easier to review a PR when you can sit down and actually talk about it in real time with the author, rather than having the ping messages back and forth interminably until you reach an agreement

In one of my first jobs, code reviews were done exactly like this, with the the reviewer (my boss) and I sitting together at one computer going through the changes. It definitely has benefits but it's still important to ensure recommendations/concerns etc. get written down and doesn't necessarily help if there's a need to go away and make significant changes based on the outcomes of discussions. But while the back & forth discussion that often occurs during review can benefit from being synchronous, it might still be some time before your coworker has available time to participate in such a session, so it's still likely you'll need something else to work on in the mean time.

Re: In Praise of Stacked PRs

#103

Post by arxanas »

is0tope wrote: post #8

I've usually kept a rule that you should avoid stacking, and if you must only one level deep. The fact that you have to stack in the first place typically suggests that PRs aren't being merged fast enough. Stacking in my personal experience usually leads to merge conflict hell as changes and PR suggestions get merged underneath you.

If you work at asynchronous/remote work company, i.e. your coworkers are in different timezones and can't review immediately, what else are you going to do? Put out exactly one code review per day until your feature is fully merged? Some things like refactoring changes can be reviewed and committed individually, but lots of feature work is fundamentally dependent on the previous work.

Stacking PRs is like pipelining for CPUs. It's efficient under the hypothesis that there aren't too many invalidations/stalls. The linked tooling `git-branchless` (I'm the author) is aimed at reducing the impact of such an invalidation by significantly improving the conflict resolution workflows.

Re: In Praise of Stacked PRs

#105

Post by wizofaus »

duped wrote: post #86
computronus wrote: post #67

Earlier quoted context omitted.

Well, I'm proceeding from the assumption that thoughtful review, which takes time, is desirable. If the situation is really that dire, then it's even more important that you ship product that works. Code review happens to be one of the, if not the most, effective ways to catch bugs and prevent disasters. It's a good idea to make the review process work for you, even and especially when the pressure is on. It's not ab…

I have a couple anecdotes to cover your question. All of them have something in common: startup needs money so you demo to potential customers or investors. Unlike in a stable corporate environment where deadlines can have flex, you really don't want to cancel or postpone a meeting to sell to a client - so those demos dates are set in stone, and if things aren't ready you will need to pull some heroics. One memorable…

Why are you doing your demos in a production environment though? How many live customers did you have that could have been seriously negatively affected by what you were deploying (assuming it almost certainly contained bugs)?

Re: In Praise of Stacked PRs

#106

Post by tomasreimers »

I am one of the authors of the tool Graphite (https://graphite.dev). We built Graphite because we missed the stack diff workflow we knew from previous job experiences at larger companies, happy to answer any questions (and if you would rather not ask in public, feel free to email me at tomas at graphite.dev).

Re: In Praise of Stacked PRs

#107

Post by fuzzy2 »

chrisseaton wrote: post #88
wowokay wrote: post #87

Earlier quoted context omitted.

No? You can create a new branch and start working on the next thing. Why would you be waiting on your PR to complete unless you didn’t split your work correctly.

What if the next thing depends on the previous thing?

Then your new branch starts at the tip of the previous one.

You can (/will have to) rebase later.

Re: In Praise of Stacked PRs

#108

Post by zmj »

is0tope wrote: post #8

I've usually kept a rule that you should avoid stacking, and if you must only one level deep. The fact that you have to stack in the first place typically suggests that PRs aren't being merged fast enough. Stacking in my personal experience usually leads to merge conflict hell as changes and PR suggestions get merged underneath you.

I stack PRs when I'm working on a piece of new code, and in the process discover one or more refactors that simplify the diff for the new code. I wouldn't start at the refactors and then wait to proceed - there's a chance they are dead ends until I know exactly what the new code needs.

Re: In Praise of Stacked PRs

#109

Post by davvid »

There is active work related to teaching "git rebase" to natively support stacked branches in the Git core currently being worked on by Derrick Stolee [1].

If you "stack" your changes across multiple inter-dependent branches it looks like "git rebase" is going to learn how to update related branches using a new "update-ref" command (alongside "squash", "fixup", "exec", etc) that gets activated automatically through a "git rebase --update-refs" command-line flag and config option.

(This is from Derrick, not me)

""" This is a feature I've wanted for quite a while. When working on the sparse index topic, I created a long RFC that actually broke into three topics for full review upstream. These topics were sequential, so any feedback on an earlier one required updates to the later ones. I would work on the full feature and use interactive rebase to update the full list of commits. However, I would need to update the branches pointing to those sub-topics.

This series adds a new --update-refs option to 'git rebase' (along with a rebase.updateRefs config option) that adds 'git update-ref' commands into the TODO list. This is powered by the commit decoration machinery.

"""

This is under active development (I don't believe the topic has been merged yet) so it's still open for feedback and refinement on the git development mailing list. Thanks Derrick!

[1] https://public-inbox.org/git/pull.1247.git.1654263472.gitgit...

Re: In Praise of Stacked PRs

#110

Post by chrisseaton »

fuzzy2 wrote: post #107
chrisseaton wrote: post #88

Earlier quoted context omitted.

What if the next thing depends on the previous thing?

Then your new branch starts at the tip of the previous one. You can (/will have to) rebase later.

I thought that was what stacked PRs are - maybe not?
Post reply on HN