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

41–50 of 230 posts

Re: In Praise of Stacked PRs

#41

Post by morelisp »

johnfn wrote: post #35
dcow wrote: post #24

Earlier quoted context omitted.

I’m not talking about the author, specifically or criticizing them directly. I’m lamenting how often I encounter people putting intense amounts of effort into arguing about how git should be used. My hypothesis is that if people understood their tools better before getting all evangelical about forcing their entire eng team to work in their style, others wouldn't have to spend so much time trying to convince them tha…

I'm not understanding how stacking PRs is inconsistent with knowing git well.

I can't be certain what dcow is referring to but there are a few things that point to confusion, or maybe an overambitious attempt at simplification.

For example, Stacking PRs keeps the author unblocked. Authors don’t need to wait on a particular change to be merged before starting to build something on top of those changes.

This will fundamentally be up to the author's git skill whether or not they are presenting the PRs to reviewers / mergers as stacked. If they're skilled git users there's little to no cost presenting them one at a time and keeping the not-yet-PRd branches fresh. If they're not skilled git users, they have no hope of managing multiple PRs effectively only be virtue of presenting them all at once.

Or: Since stacking PRs allows you to create a DAG of dependent changes, this natually allows you to manage code changes that need to be submitted in a particular order.

Assuming this is in a single repository, a fast-forward-only commit policy alone ensures this.

Or: stacked PRs use branches, and can have multiple commits in a single atomic change; stacked commits use a single commit as the unit of atomic change.

Stacked PRs usually use branches, but stacked commits also still use at least one branch and could have more. In both the commit is the unit of atomic change, because that's what a commit is.

I'll also add I find the entire language around "branchless" workflows in git confusing, not just from this author. There is no such thing; considering one special branch as "not a branch" or your local and remote and someone else's remote as the "same branch" just because they have the same name is a holdover from older/other VC tools. We don't do any new git users a pedagogical or practical favor by clinging to that view.

Re: In Praise of Stacked PRs

#42

Post by sodapopcan »

ramraj07 wrote: post #12
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.

It’s unavoidable sometimes. I get inspiration and time together rarely, I can’t wait for small chunks of code to be merged before I continue. A lot of times it’s an extremely Productive Sunday afternoon and I have 2500 new lines of code that builds a full new prototype. What am I to do?

Why would a prototype need a code review?

Re: In Praise of Stacked PRs

#43

Post by morelisp »

djur wrote: post #39
dcow wrote: post #24

Earlier quoted context omitted.

I’m not talking about the author, specifically or criticizing them directly. I’m lamenting how often I encounter people putting intense amounts of effort into arguing about how git should be used. My hypothesis is that if people understood their tools better before getting all evangelical about forcing their entire eng team to work in their style, others wouldn't have to spend so much time trying to convince them tha…

This isn't "how to use a hammer", though, it's more like "how to evaluate the completion of work", which is definitely something working teams have to agree on. git itself is almost irrelevant to the discussion, other than its native feature set having some influence on the options.

If this was just about team review workflow I wouldn't expect to find "easier to rollback" and "don’t need to wait on a particular change to be merged" to be on the list of upsides, as these are properties of the repository structure and not the review workflow.

Re: In Praise of Stacked PRs

#46

Post by avl999 »

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 you have to go in manually resolve the exact same merge conflict all the way through you PR stack in each of the PRs. This gets annoying and hard to work with.

Does anyone have a better way of dealing with this pattern of merge conflict? I've tried using git rerere which in theory sounds helpful but doesn't seem to do anything when I strike this issue.

Re: In Praise of Stacked PRs

#47

Post by morelisp »

__derek__ wrote: post #11

I'm guilty of doing this in the past, but it seems like an anti-pattern because it attempts to create a local optimum, a big no-no in the Theory of Constraints[1]. Better to find ways to sustainably ease the constraint (code reviewers' time/attention) than to find ways to create more WIP at the constraint. [1]: https://en.wikipedia.org/wiki/Theory_of_constraints

As someone coming down from a nearly 2 year fight with an engineering manager whose response to every bug was enforcing more code review rules, and every code review backlog was demanding more time allocated for code review, thank you for this comment.

Re: In Praise of Stacked PRs

#49

Post by kqr »

ignormies wrote: post #17
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.

Something I often use stacked diffs for is deprecation -> removal flows. 1. Deprecate old feature + add opt-in support for replacement 2. Make replacement default with opt-out for old pattern 3. Completely remove old feature and the opt-out functionality I can write the entire stack of diffs upfront, have them individually reviewed but still linked, and ensure they're merged in the correct order. The bottleneck for m…

But is it really effective use of your time to make changes to the code that you know won't be relevant for weeks or months? You could spend the same time on other changes that would start earning you money tomorrow, instead.

Re: In Praise of Stacked PRs

#50

Post by zeroonetwothree »

pizza234 wrote: post #16
ramraj07 wrote: post #12

Earlier quoted context omitted.

It’s unavoidable sometimes. I get inspiration and time together rarely, I can’t wait for small chunks of code to be merged before I continue. A lot of times it’s an extremely Productive Sunday afternoon and I have 2500 new lines of code that builds a full new prototype. What am I to do?

I understand that (experienced the same "problem" today), but writing "2500 new lines of code" on a Sunday afternoon is (hopefully) not representative of regular workplace conditions.

Some people enjoy their work. I think it’s fine as long as it’s a choice.
Post reply on HN