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

51–60 of 230 posts

Re: In Praise of Stacked PRs

#51

Post by kqr »

dcow wrote: post #15
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 feel similar about languages and style-guides, but I didn't want to conflate the two in my previous comment. Unless you are a royal newb and just learning to write code (which is fine, everybody has to learn), you’re probably writing code in a way that best expresses your intent or goal given your understanding of the language. Let people do that! If you know the language you can handle encountering different style…

This is something I've come to realise as I've matured as a developer. I just don't care what style you write in, if you don't use all the syntactic sugar, etc. I care where you draw your module boundaries, which units depend on which, and if you've handled edge cases, and so on. That's what's going to matter in the end. That's where I'll have to spend a lot of time figuring stuff out. Not syntax.

Re: In Praise of Stacked PRs

#52

Post by maxekman »

ridiculous_fish wrote: post #44

Plugging my own tool: if you like to cultivate a stack of commits, you'll know how awkward git makes it to edit previous commits. With git-prev-next you can run 'git prev 3' and then 'git commit --amend'. https://github.com/ridiculousfish/git-prev-next

`git rebase -i origin/main` (or HEAD~N or any other base) with the e/edit action on selected commit will do the same with only built in functionality.

Re: In Praise of Stacked PRs

#53

Post by fire »

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…

huh, shouldn't solving the conflict to pr2 create a merge commit that then also solves the conflict in 3 and 4?

Re: In Praise of Stacked PRs

#54

Post by kqr »

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…

I wish I had a better answer for you but my go-to solution as of late has been the pragmatic "I agree. Do you mind if I fix that in a later PR to avoid spending too much time on it?"

If you've built up some trust with your team, they shouldn't have a problem with it.

Re: In Praise of Stacked PRs

#55

Post by duped »

dreamdeath wrote: post #40
Supermancho wrote: post #37

Earlier quoted context omitted.

> First, think about how difficult, and time-consuming, it will be for others to digest and review 2500 new lines of code that sprung from someone else's mind. There's a tradeoff to be made. Have a feature sooner or later. Review now quickly and more carefully later, or review carefully now. Part of what development teams do, is risk assessment. Put a feature flag on it, do a demo of the branch. If it looks good, do…

> First, think about how difficult, and time-consuming, it will be for others to digest and review 2500 new lines of code that sprung from someone else's mind. I haven’t ever worked at big corp so maybe this kind of thinking is actually valuable there. But in most startups in my experience this mindset is wrong. You literally won’t have a job tomorrow (because your company will fold) if you don’t ship value-generatin…

Hence why you don't have massive PRs like that, code that is easy to review is quick to merge and quick to ship.

I wouldn't want to work at a startup that doesn't value that in their engineering culture, at least not again. More mature teams (in terms of the staff, not the business) I've been on get this and ship quickly.

Re: In Praise of Stacked PRs

#56

Post by chrisseaton »

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.

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

Re: In Praise of Stacked PRs

#57

Post by travisb »

fire wrote: post #53
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…

huh, shouldn't solving the conflict to pr2 create a merge commit that then also solves the conflict in 3 and 4?

It sounds like the OP might be using a rebase workflow. Such a workflow creates many repetitive conflicts because there is no merge commit to record the resolution point.

Using a rebase workflow with stacked/cascaded PRs is an anti-pattern that trips up people used to other git workflows where the history depth is effectively only one level deep instead of arbitrarily deep as in stacked/cascaded PRs.

Re: In Praise of Stacked PRs

#58

Post by ridiculous_fish »

maxekman wrote: post #52
ridiculous_fish wrote: post #44

Plugging my own tool: if you like to cultivate a stack of commits, you'll know how awkward git makes it to edit previous commits. With git-prev-next you can run 'git prev 3' and then 'git commit --amend'. https://github.com/ridiculousfish/git-prev-next

`git rebase -i origin/main` (or HEAD~N or any other base) with the e/edit action on selected commit will do the same with only built in functionality.

Yep, and git-prev-next is built on interactive rebase. It's just a lot faster to run `git prev`, compared to `git rebase -i HEAD^^` and then muck around in a text editor.

Re: In Praise of Stacked PRs

#59

Post by ignormies »

kqr wrote: post #49
ignormies wrote: post #17

Earlier quoted context omitted.

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.

Having the change implemented from start-to-finish demonstrates a finished and thorough design. It also allows review to happen with the complete change at the forefront of the reviewers' minds.

Kicking the completion of the implementation of a change you've started landing to an unspecified future date indicates poor engineering rigor imo. It's just begging for the change to perpetually be half-migrated and never finished.

Re: In Praise of Stacked PRs

#60

Post by krooj »

I've tried this method of taking a large change and breaking it up into smaller, more easily digestible PRs, and it didn't work. Unless you're working with folks that are directly invested in the change, you'll very quickly discover that people have memories worse than goldfish. So, it's a lose-lose situation: can't really do stacked PRs and large PRs will let bugs and other defects through.
Post reply on HN