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

91–100 of 230 posts

Re: In Praise of Stacked PRs

#91

Post by dairylee »

delecti wrote: post #83
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?

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.

Re: In Praise of Stacked PRs

#92

Post by majormajor »

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.

Sometimes?

Consider also the stakeholder who gets annoyed whenever the dev team wants to work on something that would take longer than a week to turn around, and limits the things they'll ask for to those estimated at a week. So bigger things can never get done at all, and you'll just be looking for a local maxima instead of having the chance to make more significant changes.

Sometimes it's worth it to prep and clean up as you go. Knowing when it's worth it and when it isn't is one thing that makes some devs more valuable long-term than others.

Re: In Praise of Stacked PRs

#93

Post by anderskaseorg »

travisb wrote: post #57
fire wrote: post #53

Earlier quoted context omitted.

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.

You can tell git what the resolution point is using `git rebase --onto`. This can help avoid situations where Git gives extra conflicts form trying to reapply an old version of your change on top of a new version of the same change. You can also use `git rebase --skip` if you recognize that you’ve ended up in this situation.

Re: In Praise of Stacked PRs

#94

Post by arxanas »

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

The mentioned tool `git-branchless` (I am the author) also supports `git prev 3` followed by `git commit --amend`. See https://github.com/arxanas/git-branchless/wiki/Command:-git-...

Re: In Praise of Stacked PRs

#95

Post by majormajor »

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…

If you're in a go-under-the-next-day scenario who is even reviewing your code? How big is your team, realistically?

Once you have significant runway and can hire, you need to be able to start delivering faster and faster. One person writing 2500 lines of code in bursts once or twice a week isn't going to scale. You need a team, and you need to be measuring total team throughput.

Need a new service prototype? A 2500-line PR is appropriate. But don't be surprised if it needs some big revisions - that's the point of a prototype anyway.

Need a feature? One person periodically "when inspiration and time line up" dropping 2500 lines (this is a LOT of lines in most languages used these days, even in newer, more-concise Java versions) on top of what 10 other people may be working on is not going to help everyone else move quickly at all.

If you're the person who's so busy you rarely have time to code, you need to figure out how to turn your inspirations into ideas others can execute and be the code-level experts on. A team of 10 "1x" developers is still more productive than a single "10x" developer who's in meetings figuring out the company's plan half the day anyway.

Re: In Praise of Stacked PRs

#96

Post by arxanas »

theptip wrote: post #28
compsciphd wrote: post #9

One thing I wish git did (maybe it does and I don't know how?) is to be able to say that a new branch is based off an old branch (not a commit that used to be that branches head). so I can branch a single pr in progress to start the next. Then if I change the base pr in progress (say via rebase or via squashing or the like), I can easily rebase my new commits in the new pr on top of the current state of the branch. C…

This is one of the big features from git-branchless ( https://github.com/arxanas/git-branchless/wiki/Command:-git-... ) which is recommended in the article; moving a stack (sub-tree) of commits instead of just one branch. Might be useful for you if you find the rebase flow to be painful. In particular this can be good for local prototyping where you have a bunch of functional candidates on top of some foundational re…

Also worth pointing out these commands from git-branchless (I'm the author):

- git sync: rebase all commit stacks on top of the main branch. - git restack: run after making a change to a foundational commit to automatically restack dependent commits.

Re: In Praise of Stacked PRs

#97

Post by majormajor »

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…

What am I gaining from having a stack of PRs there versus just a series of commits that I'm gonna go back and polish later, quite probably breaking into various PRs, but with some additional time and under less pressure?

When I've hacked together stuff like that I'm often making it up on the fly, I rarely have a plan in mind that lines up with how I'd want to present it for code review later. Hell, a lot of times some of the later PRs would simply remove the entirety of a failed earlier attempt!

Re: In Praise of Stacked PRs

#98

Post by arxanas »

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…

The linked tool git-branchless handles this pretty well (https://github.com/arxanas/git-branchless, I'm the author). You basically run `git checkout `, `git commit --amend`, and then `git restack`. This will rebase all dependent branches. As long as you're not using merge commits, you won't have to resolve the same conflict more than once. (It will also warn you up front whether or not merge conflicts will need to be resolved; you can pass `--merge` to start merge conflict resolution.)

To rebase your commit stack on top of the main branch, use `git sync` instead of `git merge`. Merge commits often make it so that you have to resolve conflicts multiple times.

Re: In Praise of Stacked PRs

#99

Post by [deleted] »

computronus wrote: post #67
dreamdeath wrote: post #40

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. 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…

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…

[deleted]

Re: In Praise of Stacked PRs

#100

Post by arxanas »

morelisp wrote: post #41
johnfn wrote: post #35

Earlier quoted context omitted.

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 th…

git-branchless author here. By "branchless", I mean literally using detached HEAD as the primary state of development. If you're using tools like Gerrit or Phabricator, you never have to explicitly make a branch to get your code merged. If using GitHub, then branches are unavoidable, but it can be nice to do branchless development as part of rapid prototyping (see https://github.com/arxanas/git-branchless/wiki/Workflow:-div...).

Another term you could use is "anonymous branching". This is not technically accurate in the above workflows, but it captures the essence pretty well in Git, more so than "branchless".

Post reply on HN