I may have found the biggest sin in software engineering. I wonder if you feel the same. It’s not about testing. Not about documentation. Not even about variable naming.
Committing this sin means more bugs. It means a less maintainable codebase. It means longer, lousier code review. And worst of all, it doesn’t feel that bad; it’s an innocent crime and not an obviously terrible idea. But, it is.
The sin, my friends, is big pull requests.
A big PR is the biggest sin in software engineering.
Wait! Don’t leave yet. I know, this sounds like another obnoxious Hackernews dev tip. But eliminating big PRs from your life will put you ahead of 90% of software teams.
The remedy to big PRs is, not surprisingly, small PRs (doctors hate him!). But this is easier said than done. Let’s explore the problem of big PRs, and consider strategies for making them smaller.
The problem with big PRs
As I mentioned, on the surface, big PRs don’t seem sinful. After all, a big PR implies you’ve done a lot of work. Yay for you! But sadly your big PR has three negative side effects:
- Longer time-to-merge.
- More bugs.
- Harder maintainability.
When it comes to PRs, size matters.
So, how big is too big? Well, 1000+ lines changed, certainly. But I'm skeptical seeing PRs at the 500 lines changed level, too. I'll take a stance: if your PR is above 500 lines changed, it might be too big.
Let's dig into why.
Longer time-to-merge
Time-to-merge means what you think it means: it’s the lifespan of your PR. The time it takes for a PR to merge. Time-to-merge is a function of your code review process; the longer the code review, the longer the time-to-merge.
Our goal should be to get time-to-merge as low as possible. 7 days is okay. 2 days or less is best.
If you have PRs open for longer than 2 weeks, you have a big problem. And that problem is likely that your PRs are too big.
The longer a PR stays open, the longer it takes for your user to get the feature. Obviously. What’s less clear is that the quicker you can get code into production, the better. Every dev learns that code takes on another life once it’s in production. And there’s only one way to know how your code will perform in production: ship it.
Big PRs tend to stay open for longer. For one, it’s harder to keep them up to date with the main branch. Not much worse than rebasing a massive PR that’s a week out of date. You know the feeling!
Beyond merge hell, review cycles tend to be longer.
As a code reviewer, you’ll spend more time reviewing big PRs. You need more focus. And you’ll leave more comments, of lower quality, leading to more back-and-forth with the PR author. Reviewing a big PR throws your whole day off.
All told, big PRs result in wasted time. I hesitate to use the word “waste”. But given there is a remedy (smaller PRs), this is time you can save.
More bugs
Imagine you’re reviewing a PR with 1000 lines changed across 23 files. How good is your code review going to be?
Be honest. Your code review will suck. You’ll catch fewer bugs, leave less instructive feedback, and be less patient with the author.
Nobody likes reviewing big PRs. So why would they yield a good review?
On the flip side, reviews on smaller PRs are almost always better reviews. The code review is no longer an overwhelming task. You’ll be more inclined to dig deeper into the code, more inclined to test the feature, and far more likely to find bugs.
The same principle applies to the PR author. A big PR means you’re keeping a lot of context in your head at once. We all like to think we’re good enough to do this, but we aren’t. Smaller PRs reduce the scope of the problem. With smaller PRs, you’ll take more time with code quality. You’ll introduce fewer bugs. And you’ll build better software.
Poorer maintainability
Ever tried reverting a big PR? It’s not fun.
The more code introduced in a single PR, the harder it is to revert.
And remember how big PRs yield worse code reviews? Well, that’s going to affect the maintainability of your codebase.
What about how it’s harder to author a good big PR? Maintainability suffers here, too.
Create smaller PRs
So, create smaller PRs! There are two approaches I’ve seen work well: feature flags, and stacked diffs (or chained PRs).
Use feature flags
When I learned how to use feature flags in development, I felt like I was unlocked.
A feature flag is a toggle you use to show or hide a feature in your app. Don’t want to release a feature to users yet? Put it behind a feature flag until it’s ready.
Feature flags can be as simple as boolean items in local storage, or as complex as using a third-party SaaS. Choose whatever your team will actually use.
“But Tom!”, you might say. “Why not simply have a feature branch? We can create smaller PRs that target the feature branch, and when the feature is ready, we merge it to main!”.
Many teams do this. But if big PRs are the biggest sin in software, then long-lived feature branches are the runner-up. Long-lived feature branches suffer all the same problems as big PRs. It’s painful to keep them in sync with the main branch. Your code stays out of production for longer, so you might get a big surprise when you finally merge it.
Feature flags solve this problem. Add a feature flag, and start merging small PRs behind the feature flag. When the feature is ready, turn it in and give it a test. When you’re confident in the feature, remove the feature flag.
Let’s say you had to implement the following feature:
When the user enters a value and clicks “Add Juice”, open a confirmation modal.
You could build the whole thing in one go. But then you’d end up with a 1000+ line PR. And now we know, big PRs are a problem.
Instead, you could build this behind a feature flag. In your first PR, you’d add the feature flag, along with code for the input field and the “Add Juice” button. The feature isn’t done yet; clicking the button doesn’t do anything! But that’s okay because we’re using a feature flag. Then in the second PR, you’d add the code for the modal. You might even have a third PR to refine the UX further.
Instead of a thick 1000+ line PR, you’ve shipped 3 small PRs. Woot!
Feature flags are your friend.
Stack your PRs
Sometimes feature flags are overkill. Sometimes all you need is to stack your PRs.
Stacking, or chaining PRs is what it sounds like: instead of one big PR, you could have three small PRs linked in a chain. The third PR depends on the second, and the second PR depends on the first.
This works well in situations where you need to refactor a bunch of code before building your feature. Instead of one big PR that includes the refactor and the feature, create one PR that only includes the refactor, and a second PR that includes the feature.
This way, each PR is reviewed and tested independently. Your time-to-merge will certainly be shorter, and hopefully, you’ve received a better code review.
If you’re really lucky, you might not even have to chain the PRs. If you can find a way to break up your big PR into multiple independent chunks, then you have achieved mastery. Pass your on-call shift to someone else!
Big PRs are a problem. They increase time-to-merge and lead to a buggier and less maintainable product. Do yourself and your code reviewers a favor. Create smaller PRs, with the help of feature flags and stacked PRs. Sometimes you can’t get around creating a big meaty PR. No worries. It just means you owe your team a small PR next time.
I tweet about building software in web3 and crypto - follow me over there: twitter.com/aeolianeth