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

201–210 of 230 posts

Re: In Praise of Stacked PRs

#201

Post by Sol- »

arxanas wrote: post #132
Sol- wrote: post #128

We have some people in our company that use tools like Graphite to implement this pattern on top of Github and I don't know if I'm a big fan. Maybe just a matter of developer discipline, but in my experience people tend to create large stacks of 3+ PRs that then take a while to resolve. Yeah sure, without these tools the code would also exist somewhere, but at least you don't have your pull request list full with PRs…

> Probably some arcane git magic to (interactively) rebase branch There is not really a command for that yet, short of adding a bunch of `exec` steps to your interactive rebase manually. See https://news.ycombinator.com/item?id=32217204 for an upcoming command. You might enjoy using https://github.com/gitext-rs/git-stack , which specifically tries to let you manage stacked branches locally while not exposing tons of…

Thanks for the reply and the pointers, very helpful!

Re: In Praise of Stacked PRs

#202

Post by Sol- »

tcoff91 wrote: post #174
Sol- wrote: post #128

We have some people in our company that use tools like Graphite to implement this pattern on top of Github and I don't know if I'm a big fan. Maybe just a matter of developer discipline, but in my experience people tend to create large stacks of 3+ PRs that then take a while to resolve. Yeah sure, without these tools the code would also exist somewhere, but at least you don't have your pull request list full with PRs…

https://git-ps.sh/ This git patch stack tool is really nice. You don’t really use branches at all with it. You just build up a stack of atomic commits. At any time, you can request review of a patch, and it will cherry-pick the commit onto a new branch off of master and create a PR for it.

Interesting, thanks. Will give it a try!

Re: In Praise of Stacked PRs

#203

Post by slimsag »

davvid wrote: post #109

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…

Wow, this is incredible and would make myself and so many people I know super happy! This is awesome work!

Re: In Praise of Stacked PRs

#204

Post by krobelus »

WookieRushing wrote: post #193
avl999 wrote: post #46

I do stacked PRs at work, they work great until someone suggests an invasive change in a PR lower down in the stack. Does anyone have ideas on how to deal with merge conflicts in this type of flow? For example let's say I have the following stack of PRs PR1 -> PR2 -> PR3 -> PR4 The reviewer reviews and suggests a change in PR1, this change causes a merge conflict in PR2 and therefore in PR3 and PR4 as well. And then…

Mercurial solves this 90% of the way with hg absorb: https://gregoryszorc.com/blog/2018/11/05/absorbing-commit-ch... Its extremely close to magic. You make all the requested changes on the PR4 and type hg absorb and it'll figure out which commit in your stack each change belongs to. I've used this to work on lots of stacks 20 commits deep in mercurial.

The equivalent to "hg absorb" for Git is https://github.com/torbiak/git-autofixup

Re: In Praise of Stacked PRs

#205

Post by JD557 »

arxanas wrote: post #103
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…

One alternative that I sometimes do is to continue the work on a new branch (just as if I was doing stacked PRs), but not send any new PR until the first PR is merged (or at least approved).

Mostly because stacked PRs are usually not ready for review until the base PR is reviewed, and are probably going to get rebased and require some refactors. I've had cases where the base PR had so many changes that I just started over on a new branch.

I know that GitHub now has draft PRs, but I still think that (unless you really want someone to take a look at the draft while the base PR is not merged), it might be better to not make people waste time looking at code that might suffer heavy changes.

Re: In Praise of Stacked PRs

#206

Post by crubier »

Groxx wrote: post #176
dimal wrote: post #156

Earlier quoted context omitted.

How is reviewing individual commits a good thing? Maybe I’m misunderstanding something, but my branches have tons of micro commits that ultimately lead to a larger change. Reviewing each individual commit in that type of branch seems like it would be very tedious, and not useful. The reviewer would be forced to follow my entire process, with all the dead ends and refactors along the way. Seems like it would take a lo…

With this sort of system you generally squash your commits together to form one-per-reviewable-thing. Or at least you rewrite things a bit to make it approachable. E.g. I split things apart so one commit is an obviously-correct refactor and the later ones are pure changes. I understand some of the "no, merges are superior because they retain all history" proponents (not claiming you are one, just aiming to head off t…

“They ruin the ability to bisect” is something I find myself constantly fighting. Just use “—-first-parent” in git log!!

Re: In Praise of Stacked PRs

#207

Post by atq2119 »

majormajor wrote: post #90
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?

Two top-level scenarios here: * you're working on the same part of the code * you aren't working on the same part of the code The second scenario is common but also trivial here since you can just have parallel branches going, so I'm gonna assume more the first - working on something that's building on top of what you just put up for review. Let's say the review is done in 2 hours. If you're already done with the fol…

> Let's say the review is done in 2 hours.

One of the projects I work on is a large, well-known open source project. Reviews that take days are pretty common.

> If you're already done with the followup, IMO you may be erring too far on the side of "small PRs."

There have been times when I created a sequence of three or four separate commits within under half an hour.

This was on projects where test coverage is tricky (think hardware interfaces) and keeping changes small was motivated a lot by better bisectability.

Clearly, software development is a pretty broad field with lots of different experiences.

Re: In Praise of Stacked PRs

#208

Post by oehpr »

dcow wrote: post #7

I’m going to be a little crass for a moment: people don’t know how to use their tools! Why are we even talking about “stacked PRs” and “branchless” workflows?! It’s clear the author is documenting their git learning process and that’s great, really. But I’m just surprised how pedantic people get about forcing others to use a tool in a way that caters to their own limited understanding of it. Or, even worse, caters to…

I mean, in this case, no one's forcing. More like advocating.

And git is a very flexible tool with a lot of generic functions. Pull requests themselves are a prescriptive style of using those generic functions. A "Branchless" workflow is not the author failing to understand git. It's using those functions differently.

And I fail to see how it's worse that workflow constraints may be placed on a very generic tool like git to work better with more specialized tooling in other places.

"I can put any text in a commit message, so why do I have to start it with this specific text?"

"Because that's how our organization tracks work, and the ability to track historic work in this way is more valuable to us than you being able to write whatever you want at the start of a commit message"

Re: In Praise of Stacked PRs

#209

Post by duped »

wizofaus wrote: post #105
duped wrote: post #86

Earlier quoted context omitted.

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

Zero, but only one of those demos actually touched production.

Re: In Praise of Stacked PRs

#210

Post by delecti »

dairylee wrote: post #91
delecti wrote: post #83

Earlier quoted context omitted.

If your second PR is ready before the first PR is merged, then two of the likliest outcomes are that either PR reviews are taking too long, or the second PR is small enough that it could have just been part of the first. Alternatively, the review is taking a long time because the first PR was bad/controversial, in which case the assumptions of the second PR might need to be reevaluated anyway.

Neither of those cases need to be true for a second PR to be ready before the first has been merged. For example you do your first PR, mark it ready for review. While doing it you notice there's some refactoring you could do to some tangentially related code. It's very conceivable that the second refactoring PR could be ready pretty quickly.

There's a reason I said "two of the likliest outcomes", and not "the only two possibilities".

Yes there are other situations, but in my experience, the ones I mentioned are the most common.

Post reply on HN