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

[majormajor]

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!

[wizofaus]

Why are you doing your demos in a production environment though? How many live customers did you have that could have been seriously negatively affected by what you were deploying (assuming it almost certainly contained bugs)?

[duped]

Zero, but only one of those demos actually touched production.

[computronus]

Thanks for the anecdotes!

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