[is0tope]

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.

[arxanas]

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.

[isoos]

> what else are you going to do?

Depends on the team and the product. My personal approach is to have 2-3 larger things to work on, so while I wait for reviews on one, I can switch and work on the other. This usually means minimum 1-2 weeks of planned work, sometimes even more, without being blocked on reviews. If everything is blocked, then it is time for some code health cleanup, refactoring and fixing those TODOs that are just lingering around, and also nudging the reviewers...

[JD557]

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.

[smaudet]

> what else are you going to do

You can also change the definition of done - something I try to do when a task gets too large, spin off new sub-tasks, etc. The first feature doesn't quite work right, but the merge differential is usually smaller for tweaks or bug-fixes than it is for major initial features.

You can spend a lot time planning how best to break up your work, and sometimes you may be able to do that successfully, but often times it you are constrained not by git but by your tracking tool (jira, trac), which impose somewhat artificial sizes on code tasks....

And really that's the whole thing stacked prs or commits are trying to solve, minimize the merge differential.

[ignormies]

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 merging isn't in the review process, but in the deprecation. It wouldn't make sense to land all three of these changes as fast as review/merge would allow; that would skip the deprecation period.

[wpietri]

Ooh, good point. I'm suspicious of stacked diffs generally, but this seems like a good example where we truly don't expect to learn anything after the initial deploy.

[kqr]

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.

[wpietri]

For sure. One of the things I learned from the Lean folks was to look for inventory; it's one of the 7 Wastes. [1] In physical manufacturing, it's pretty obvious, because it's physical stuff sitting around on the journey to becoming actually useful.

With software it can be harder to notice because you don't have to make room for it. But in essence it's the same deal; it's anything we have paid to create that isn't yet delivering value to the people we serve. Plans, design docs, and any unshipped code.

There are a lot of reasons to avoid inventory, but a big one is that until something is in use, we haven't closed a feedback loop. Software inventory embodies untested hypotheses. E.g., a product manager thinks users will use X. A designer thinks Y will improve an interface for new users. A developer thinks Z will make for cleaner, faster code.

Both large pull requests and stacked pull requests increase inventory. In the case of incorrect hypotheses, they also increase rework. I could believe that for a well-performing team stacked PRs are better than equally-sized single big PRs, in that they could reduce inventory and cycle time. But like you, I think I'd rather just go for frequent, unstacked, small PRs.

[1] e.g. https://kanbanize.com/lean-management/value-waste/7-wastes-o...

[vlovich123]

I have stacked diffs sometimes when the rework is large. I want to make sure that I know the full story sounds the change I’m making because I’m forced to think about that upfront. What refactoring was needed? Was it actually needed? What new path do I carve out in the code or how do features interplay? Broken tests with good coverage tell me if I made a foundational mistake. Even if I decide to throw away the work because it was a dead end (rare) the team will have learned something by me explaining what didn’t work out. More often, I’ll need to go back and clean up. But I save significant reviewer time by doing that before putting up random prs one at a time that are not well understood. With the exception of very simple work, stacked reviews generally save significant time. You get reviews of non objectionable prs. Coworkers can see a bigger picture if that’s helpful to understand the context of the change that’s still coming into shape. It actually reduces merge conflicts because, for example, you can enable a refactor that everyone agrees needs to be done and land that. Then your conflict space is smaller.

Small prs don’t need it of course but complex features benefit from shaking out things earlier. Commit more than 100 lines are really hard to review (lots of anecdotal and empirical research). If you’re not reviewing small commit by small commit, the reviews are easily missing things. A single PR that’s 800 lines adds review time to go commit by commit. If you can merge the non objectionable stuff, the reviewee gets to feel a some of forward progress and fewer merge conflicts (eg someone lands a refactor before some simple change of yours vs your simple change handed before and you made it the person refactoring their problem where it belongs)

[bosie]

> One of the things I learned from the Lean folks was to look for inventory; it's one of the 7 Wastes. [1] In physical manufacturing, it's pretty obvious, because it's physical stuff sitting around on the journey to becoming actually useful.

I guess this is not true anymore post covid outbreak? Pretty sure a lot of companies would kill to have inventory of their raw materials right now...

[chrisseaton]

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

[majormajor]

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 followup, IMO you may be erring too far on the side of "small PRs." If you aren't, you just rebase on top of whatever changes had to be made, if any, to the first one, once it lands on the primary branch.

If, on the other hand, the review isn't done for 2 days... then that's a PR turnaround time problem for sure.

But I strongly disagree with the people saying "multiple dependent PRs suggests the work wasn't split up properly" - there's nothing worse than a mega-PR of thousands of lines for the sake of doing a "single feature" all in one shot vs having to possibly pause and rebase periodically after review. It's even more painful when this mega-PR requires fundamental changes that could've been caught earlier, but now will take longer to adjust, and then will stay open for a while and likely result in merge conflicts as a result.

[perrygeo]

By pinging people on slack and interrupting them for a review asap? Or context switching to an unrelated part of the code in parallel while you wait? Or reducing the frequency of reviews and make a mega-PR every couple weeks?

I've seen no solutions, only tradeoffs but I'm curious if anyone has a tried and true way to avoid this traffic jam scenario.

[nightpool]

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

[delecti]

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.

[californical]

You can branch off of a PR, but someone should review and merge your first PR before your second is ready to be up in a PR again.

Also, trying to make units of work so that they don’t need to overlap like that can be useful too

[wowokay]

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.

[wpietri]

One way this can work is pair programming. Especially if you also practice frequent pair rotation, you can get plenty of eyeballs on code in a timely fashion. That's my preference.

Another way is that you next work on something different enough that it doesn't need to stack. E.g., reviewing pull requests.

[gregmac]

I sometimes stack retroactively. I work on something experimental, then realize it'll be easier to review if I break it down.

This often means first a code cleanup that doesn't change any functionality yet, but makes the later changes simpler - eg; removing dead code, removing unnecessary abstractions/interfaces/layers, upgrading external packages.

Sometimes I recognize this early, and will specifically make a branch for this. If there's changes from the review I'll rebase my next branch(es) on top.

Often I don't see this will be needed until later (or rather: I'm focused on the change itself and not the PR experience) and so I'll interactive rebase to put all the refactor commits first, and make a PR for those.

Both cases mean I already have a branch built off an unmerged PR. Sometimes even before the PR is published, actually. I don't see any particular problem with this. My development style and speed is independent of the speed PRs get merged; the ability to rebase makes this a total non-issue.

[jillesvangurp]

The trick with long lived branches is to merge upstream changes frequently so you don't fall behind and keep your branch in a mergeable state. Git was designed for code bases much larger than what most people deal with where long lived branches are just a reality.

Like the Linux kernel for example. You don't get to push half finished work with projects like that. Actually, you don't get to push at all. There is only pull. Some branches exist for many months or even years and there could be entire teams collaborating on them. The one absolute certainty you have on such branches is that there is an absolutely insane amount of upstream changes all the time. The only way to stay on top of that is to merge those changes often.

You can think of the Linux kernel git as a decentralized network of stacked branches that have a few central people pulling changes from branches they have reviewed into their own branches with the one that Linus Torvalds maintains as the ultimate branch on which releases get tagged. The vast majority of changes land to his branch via multiple layers of other people, each with their own branches and each adding their own reviews. Effectively all changes Linus Torvalds integrates are stacked. And he doesn't integrate them unless they are stacked properly with nice clean histories.

You could say, git was explicitly designed to do stacked branches at scale. So, it's kind of ironic that people are re-discovering this as a thing. It always was intended to be used like this. It's been used like that since the very beginning.

[ramraj07]

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?

[computronus]

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. So you will end up waiting anyway, for even a small part of your work to be merged.

The work of breaking up a big, inspired chunk of work into small pieces helps you learn more about it, and the perspective can reveal improvements that weren't obvious in the initial effort. You might notice those yourself, or reviewers may. The final outcome will end up overall better for it, so spending that time is worthwhile.

[pizza234]

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.

[sodapopcan]

Why would a prototype need a code review?

[dan-robertson]

One advantage of ‘stacking’ is breaking up review into more logical units, eg

1. Introduce new test exhibiting bug

2. Introduce bug fix and update the test

If 1 and 2 are reviewed together you have less evidence that the test actually shows the bug being fixed.

[dtech]

You can still have them in 2 commits, and configure your CI to build both of them, and the first 1 should fail.

We actually have a rule that there must be 1 commit just introducing a test that fails on CI for bugfix PRs.

[YZF]

Ideally your CI workflow prevents merging something that breaks the tests. Also now if the first review merges but the other doesn't for some reason you've broken everyone else which is bad.

EDIT: at least in rebase workflows which is what I'm used to. I guess in merge workflows this works.

[cornel_io]

But once you merge 1, your test suite is broken, and many/most companies wouldn't allow that.

[BerislavLopac]

My personal rule of thumb is to rebase the working branch as soon as the main has been updated; or at least merge main if the changes are too complex.

[zmj]

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.

[barrkel]

If you subscribe to the idea of small PRs (dozen or so diff lines at most) and code ownership then you'll want to split PRs up and have them reviewed by different people. In this mode blocking on code review interrupts flow, and it's not unusual in a bigger company for the owners of some code to be in a different time zone.

[mrkeen]

> The fact that you have to stack in the first place typically suggests that PRs aren't being merged fast enough

Yes. Whenever you inspect a proposed solution, you should hopefully find the problem that it solves.