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

[jacobegold]

The advantage of using stacks, though, is that you can keep focus on what you're working on, or even still work on 2-3 things simultaneously but work on multiple dependent pieces of each at once. Maybe I'm biased because I've used this workflow my whole career, but I think everyone can benefit from trying out a stacked PR workflow.

Not to mentioned getting reviews for smaller, atomic changes is just SO much easier. Even on a team where everyone is using a stacked workflow, if anyone submits a larger PR (especially more than a few hundred lines), you can see how the smaller PRs submitted in the same time, often in the same stack, get reviewed much faster.

[rendaw]

I don't think anyone wouldn't agree that being able to work deeply on a single async task would be a great benefit, but your reply completely ignores the issue OP brought up:

> Stacking in my personal experience usually leads to merge conflict hell as changes and PR suggestions get merged underneath you.

That's been my experience too.

And beyond the technical issues, the deeper you work on a single issue the more at risk you are of the simple issue of finding design or requirements issues in the base MR that require going a different direction and discarding the whole stack. So even if you can somehow avoid conflict issues, stacks are still dangerous.

[pluiz]

That's the approach that I've been doing for a while now as well, I feel way more productive when a switch things that I'm working on and then come back later, and it also helps to keep it up with the time that my team-mates are available to review my changes...

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

[tablespoon]

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

It totally is, because it doesn't wastefully discard the mental context needed to make the follow on changes. Task switching unnecessarily incurs significant costs.

> You could spend the same time on other changes that would start earning you money tomorrow, instead.

Maybe in some bare-bones startup context that can't afford to think beyond next week, but most organizations aren't like that.

[majormajor]

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.

[smaudet]

Agreed, sizing is one of the things that can set apart devs, either due to experience, knowledge, or some combination of both.

[ignormies]

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.

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

[wpietri]

It sounds like you're talking about a bunch of different cases, and I'm having trouble untangling them.

If there's a simple refactoring everybody agrees is good whether or not your overall goal ends up making sense, then yes, by all means merge that. But that doesn't require stacking unless your review process is slow. In which case I still think the right solution is to speed up review, not to stack.

For the cases where we don't know the full story, my first thought is that we never know the full story. So there I try to instead find the smallest unit of work that everybody agrees is a step forward.

When that's not possible, where the unit of work still seems pretty large, instead of breaking that up into a bunch of stacked diffs that shouldn't be merged until we really understand something (which to me sounds like a large PR in disguise), I think a better option is a spike, where we intentionally do a quick, throwaway version of some change as a way of learning about the change. Instead of trying to do good code along the way to good understanding, we just go for the understanding. Once we have thrown out that scratch code, we then go back with our new knowledge for a proper PR.

So I'm still not seeing where I would use stacked diffs, except in this case here: https://news.ycombinator.com/item?id=32215346

The premise of stacked diffs seems that we won't learn anything significant from reviewing or deploying code. (If we did learn something valuable, then the things stacked on top could be up for a lot of rework or might be thrown out altogether.) I think that has a lot of bad effects, but one of the biggest for me is that the bigger the inventory of code (whether in one big PR or a stacked set of smaller ones), the more a reviewer will feel obliged to say, "LGTM" and let it go, because they know there's not much point in saying, "Actually, I think this whole thing could be better approached by X."

So like you I'm entirely for small, reviewable lumps that are easily merged. I just think they should be then reviewed quickly, so that stacking or agglomerating is unnecessary.

[vlovich123]

> unless your review process is slow. In which case I still think the right solution is to speed up review, not to stack.

This feels like a critically important point to your position but there's no actionable advice provided on how to achieve that. FWIW I've found stacked PRs do speedup reviews because it pipelines the work. Pipelining something removes bubbles (in this case time spent waiting on review) from forming.

Simpler parts of the work get eyeballs from more junior engineers who feel more comfortable approving smaller / simpler PRs (& other people feel confident in that). Trickier stuff is left to the smaller pool of people who have the appropriate context / skillset. If you're waiting on landing 1 PR at a time, then you're serializing the review flow which means your total time on the PR is "time spent writing + time spent reviewing 100% of the code". If you pipeline your stacked diff, then potentially you could get ~80% of the code reviewed & landed by the time you finish the more complex pieces. Then you're left with "time spent writing + time spent reviewing 20% of the code". Additionally, by putting up the commits early, you're letting other people fit smaller reviews into their schedule more easily vs "here's a PR with 5000 lines of code changes" which is a monstrosity to review (i.e. quickly runs into mental fatigue issues / quality of the reviews can easily degrade, especially if commit hygiene wasn't practiced).

Have you actually tried stack PRs with a proper review process & good commit hygiene? This is one of those "try it before you knock" it things.

[wpietri]

Again, I think you're tangling together cases. And also tangling arguments. You can't argue from theoretical gains in an imagined optimal case and then say the only argument that matters is the empirical one.

If your point is that in some existing organizations stacked pull requests are better for a specific engineer's experience than doing all the related code in a big blob, I certainly believe you.

Similarly, there are manufacturing shops where reducing inventory in line with Lean approaches doesn't work out of the gate, because there are other organizational problems/constraints that have to be dealt with first. For example, you might need a large buffer of component X at stage Y of a manufacturing process because upstream quality issues mean that a smaller buffer would cause frequent stalls at stage Y. First you have to fix the upstream issue before you can cut stocks there.

So are stacked pull requests the optimal choice for some specific person on some specific occasion? Sure! I'll take your word that's the case for you. What I'm saying is that I think they're an indicator that there is some systemic problem that could be resolved so stacked pull requests and giant pull requests both become unnecessary.

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

[wpietri]

It is still true. The lean analysis of waste splits things into "necessary waste" and "pure waste". For a particular place and moment in time, there will be some waste that you can't remove without harming production significantly. That's necessary waste. The goal in the long term is to reduce total waste by finding ways to make some bit of necessary waste unnecessary.

It's true that pandemic supply chain issues have change the level of necessary waste in a lot of supply chains. But that doesn't make inventory good. Often production halts not due to everything being missing, but a shortfall of just one input. A company might mistakenly react by stock up on everything, but that still won't solve the shortfall of the critical component.

So should the just stock up on the critical component? Go get a year's backlog of that? If everybody does that, that will cause a shortfall all on its own, as when everybody did panic buying of toilet paper in 2020. And then when supply chains straighten out, then the stockpile is back to being unnecessary waste. So I don't think there are any simple answers there.

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

[atq2119]

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

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

[wowokay]

Yes, complete your story in a single PR, if the next story is related and requires your changes your work wasn’t split up properly, plus in that scenario there is a good chance other devs have to many dependencies on other devs work which is a recipe for disaster!

[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

[wizofaus]

In one of my first jobs, code reviews were done exactly like this, with the the reviewer (my boss) and I sitting together at one computer going through the changes. It definitely has benefits but it's still important to ensure recommendations/concerns etc. get written down and doesn't necessarily help if there's a need to go away and make significant changes based on the outcomes of discussions. But while the back & forth discussion that often occurs during review can benefit from being synchronous, it might still be some time before your coworker has available time to participate in such a session, so it's still likely you'll need something else to work on in the mean time.

[nightpool]

> and doesn't necessarily help if there's a need to go away and make significant changes based on the outcomes of discussions.

See, I disagree, because this is absolutely the place where it helps the most—you can now go away and keep working on the same task without having to context switch to anything else or remember where you were or what you were working on. So you're never in a state where you're blocked and can't work on your ticket, and you don't have anything else to do or have to pick up another ticket and start learning about a whole completely separate problem. (And yes, definitely everything still needs to be written down, it's important to walk away knowing which changes you need to make, and why!)

> it might still be some time before your coworker has available time to participate in such a session, so it's still likely you'll need something else to work on in the mean time.

In teams I've worked on, the expectation is that an engineers highest priority is always unblocking another engineer, so this very rarely happened. Unless they had an interview to go to or some kind of meeting—and in that case, you could always just ping somebody else.

Obviously it's a style of work that relies really heavily on everyone sharing the same timezone and work hours, but it works really well to eliminate time lost to context switching and minimize the amount of time engineers spend blocked waiting on someone else.

[wizofaus]

But if you're expecting another dev to always be available to help you with a review, then they're the ones having to "context-switch", interrupting their own work. The reality is context-switching is something that comes with the territory if you're working as part of a team developing software. Which isn't to say there aren't opportunities to minimise the disruption it causes, but the idea that you can more or less eliminate it seems like wishful thinking. Perhaps there's a case for minimising it for more junior devs that aren't as capable of dealing with it, though it's equally possible younger brains are better at it!

[nightpool]

I think maybe we're using different definitions of the term "context-switch". Certainly it's an interruption, but I don't really think that sitting down with another engineer to do a focused code review where they've already written out a PR description and thought hard about the problem is comparable to starting a brand new branch or picking it up after a while away and trying to juggle 2, 3, or 4 in-progress tickets.

[daniel-grigg]

This sounds like you’re just externalising the costs of integration of your changes on to other people.

Generally we want to reduce any accidentally complexity and the way to achieve that is to reduce the latency of code reaching production. So for example you want to minimise PR/branches in flight ideally to 0. So you should be asking how you can reduce them - eg trunk based development, omitting asynchronous code reviews etc.

[nightpool]

> This sounds like you’re just externalising the costs of integration of your changes on to other people.

Huh? Nothing I said has anything to do with "the cost of integrating changes". I'm saying that it's more efficient for developers to work on one single task at once, instead of trying to "multi-task" among a variety of unrelated changes, and that synchronous code reviews help with this because they minimize the amount of time a developer spends blocked waiting for feedback from the rest of the team. Certainly this also has the benefit that you get fewer merge conflicts and have fewer branches in flight, but I don't know why you're saying that synchronous code reviews are "just externalizing the costs of integration [...] on to other people". That doesn't make any sense.

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

[dairylee]

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.

[delecti]

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.

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

[chrisseaton]

What if the next thing depends on the previous thing?

[fuzzy2]

Then your new branch starts at the tip of the previous one.

You can (/will have to) rebase later.

[chrisseaton]

I thought that was what stacked PRs are - maybe not?

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

[celtain]

Isn't this exactly why you should use stacked PRs? That's what it looks like when I break a big piece of work into smaller pieces.

What's the alternative anyway? If you don't want me writing 2500 lines of code in one area, would you rather I write 10 250 line PRs in 10 different parts of the codebase instead? Is that supposed to be easier to review?

Or is the rule just "don't write a lot of code in any short period of time"?

[morelisp]

> don't write a lot of code in any short period of time

Depending on the team this may be a completely reasonable policy.

[bpodgursky]

(if you are on this team, run away as fast as you can)

[morelisp]

No, there’s no reason to run away from a team of overenthusiastic junior developers. Teach them instead and you’ll get a team of enthusiastic skilled developers, you just need to make sure they don’t make too much of a mess in the meantime.

[Supermancho]

> 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 a quick once-over to see if it's interactive with any limited resources, merge it in, make a ticket for a re-review later.

[dreamdeath]

> 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-generating product yesterday. But you’re going to worry about how inconvenient it will be for some other developer to review your PR?

[jcpst]

To offer an alternative perspective: I haven’t ever worked at a startup.

If someone wanted a PR review for 2500 lines, I would tell them no. I absolutely expect a developer to think twice when trying to make a change that large in one go.

When your corporation’s billion lines of code are the engine that helps generate $30,000 a minute, you don’t just say “meh, looks good to me.”

Furthermore, this is multiplied on my team, which is the framework team. A mistake in our auth middleware, http client, etc, could easily turn into a mistake on 50+ other teams.

[duped]

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.

[majormajor]

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.

[computronus]

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 about humongous reviews being "inconvenient". A drop of thousands of lines of new code takes a long time to review thoroughly, whether the company is at risk of folding tomorrow or a stodgy enterprise with decades ahead of it. If you have to constrain review time - or ditch reviews altogether, why not? - you can, but there are consequences regardless.

I haven't worked in early startups, but I haven't only worked at large corporations either. I hope that most of us, most of the time, aren't less than a business day away from unemployment, and so there's usually time for code reviews. (If not, there's a lot of useless material about it!)

Without sarcasm, and potentially getting off-topic, I would like to hear stories about how a startup survived impending doom with heroic, fast shipping of product that set aside a lot of time-consuming processes. What were the most crucial steps to keep? How was the technical debt repaid?

[duped]

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 one was when three of us spent the hours leading up to a demo disabling automation and deploying to production by hand. What that meant was disabling a bunch of tests and code that checks those tests succeed in order to get the code out the door. We spent the next week cleaning that up. Sometimes you need to test in production.

Another was actually a demo for the same client that set up that meeting. This time we had heard from the initial introductions they had a specific, very nasty problem and we could ostensibly solve it (that was true, we could, but the product at that point had no facility to do it). So I spent I think three days hacking together a solution that would solve the problem for the contrived demos we would show. That solution had atrocious performance characteristics so we wound up spending a few story points over the next two sprints to optimize and refactor the guts.

Yet another demo was an integration. We had scoped out the general feature (which was magical when it worked) but it required a large amount of effort to coordinate with the original software vendors to get the data we would need. But those vendors have a tool with a free trial install which had a bunch of the data in XML, so I spent a day reverse engineering the schema and a second day parsing into our internal data model which we demoed on the third day to show how our product could solve that particular problem. We never wound up getting data from the first party company, and the parser got rewritten in a different language, and the backend that does the stuff is planned to be replaced in the next few sprints once it gets priority.

So TL;DR you have a demo scheduled, you hack together a thing that can be presented, then spend your time reimplementing or refactoring the demo code into something maintainable.

It helps to have a stack you can really quickly iterate on. And management that understands demos aren't suitable for production.

[Supermancho]

This isn't going to get me internet points, but after a few decades, I like to get my experience down.

> Well, I'm proceeding from the assumption that thoughtful review, which takes time, is desirable

Ofc it's desirable. Nobody is arguing this.

> If the situation is really that dire, then it's even more important that you ship product that works.

That is simply incorrect. Broken features (in innumerable shades) are debuted all the time to secure future investment in various ways which are not just financial. Sometimes it's community confidence, sometimes it's leadership clout, etc. Betas are a thing. This fear-mongering about 2500 lines of code is pearl clutching that hobbles large organizations that either fail to deliver or fail to deliver stability, regardless of their processes that drags on development for 4x or 10x, which they have misplaced confidence in.

> I hope that most of us, most of the time, aren't less than a business day away from unemployment, and so there's usually time for code reviews. (If not, there's a lot of useless material about it!)

There is lots of useless material about it. The number one rule is "don't interrupt the flow of money" but that rule doesn't extend to every project forever. Large organizations get less and less efficient over time (see rule 1) because perfect is the enemy of good. It's a good tradeoff, when you have an organization that can't keep up with how much money they are making to start piling on additional process because you don't know where your weaknesses are so anything that goes wrong gets an additional check. Most orgs are not in this boat.

If you don't look like you're making your money's worth, even if you can explain how some side-process adds value, someone starts looking for your replacement and eventually they will find one and you're gone. That's true, regardless if you work at some mom and pop shop or Amazon. You're always one day away from unemployment, even if you don't know what day the chain of events started. So code reviews that block features can hurt you and have put a lot of people out of work, I can attest. Ofc we're talking about more and more extreme situations, but the idea is the same. Code reviews are a scale and that scale has a tradeoff.

The gut-check is when a company has a merger/acquisition deal. The team doesn't have a lot of time to vet the other company's codebase. You do your best to have your info meetings, try to run parts of the code with data, make your recommendations, and the escrow eventually closes. 2500? How do you think you'll do when looking at millions of lines? At some point, you will be forced into black box testing and realize that this is what matters foremost. Look at the interface, look at the expectations, look at the data. After that, the rest is gravy that can be addressed later.

[baq]

If you're the only developer for quite a while... otherwise reviewability is a thing PRs must be optimized for or your super-duper-important feature won't get in.

Either way you should be looking for a new job, because what you're describing is quite unsustainable in a team of more than one person working on the same thing.

[jacobegold]

But you can still construct stacked PRs out of the 2500-line change even you write it all at once -- just go back and split the changes into a series of PRs that makes sense. Much easier for your reviewers to get through, so it'll be able to ship faster.

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

[ramraj07]

No one forces me to. No one asks. Sometimes I’m just inspired and would rather write this code than eat.

[zeroonetwothree]

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

[NateEag]

It may be fine.

Keep in mind, though, that humans are not perfect and some choices are unwise.

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

[chrisweekly]

That's interesting, explicitly to require that bugs be reproduced in CI. It makes sense in theory, but in praxis (IME) CI systems tend to be overtaxed / underprovisioned - meaning this extra burden might be questionable. /$.02

[dtech]

Given the cost a typical CI system vs the salary cost of a typical dev, the business case should be fairly easy that devs always should have CI capacity available.

[nemetroid]

The same burden is present when splitting the two commits into two PRs.

[chrisweekly]

Actually no, the burden I referred to was -- unrelated to stacked vs serial PRs -- specifically the GP's practice of requiring that tests fail in CI.

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

[dan-robertson]

Yeah there’s two solutions:

1. Merge #2 into #1 then merge the result, but this can obscure your history

2. Don’t make the test fail. Eg expect the incorrect behaviour and leave a comment explaining why it is incorrect and what the correct thing should be.

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