Please write smaller Pull Requests!
A desperate plea from a humble software engineer
Doing Pull Request reviews is an essential part of being a programmer working as a part of a team. Giving reviews promptly helps the teamwork quickly and giving reviews well helps keep the quality of the code-base high.
However, I’d be lying if I said I leaped at the chance to give reviews. They require you to context switch into someone else's work and try to think of things they’ve missed, edge cases, and style improvements. It’s mentally taxing work.
That’s why it’s especially frustrating when I open a PR to review and see “30 Changed Files”. This article explains why I think big PRs are a bad idea and some strategies for avoiding having to create them.
Why you Should Write Smaller PRs?
Firstly though, let me justify my hatred. Here are reasons why you should aim to have only a small amount of changes in your pull requests:
Get More and Better Feedback
I’m definitely quicker to review PRs sent my way that are small. When only a small change is being made I also find it easier to keep everything in my head and think through what’s being done. It isn’t just me; reviews on my PRs are always more forthcoming and of a better quality when my PRs are on the smaller side.
Change Less, Break Less
Bugs are always going to pop up during active development but when you make smaller changes there is a better change your reviews will spot what's wrong. When a bug does slip through, smaller PRs also mean that it’s easier to narrow down what's causing the behavior.
The Dreaded Merge Conflict
Merge conflicts are a pain to resolve and can lead to introducing bugs. If your PR is more constrained in scope it’s less likely you’ll clash with another developer's changes.
Don’t Waste Time
Often PR review is the first time you get another pair of eyes on your solution to a problem. Getting part of that solution ready to check-in quickly is a good way to verify that your approach won’t be rejected.
Several times I’ve spent the best part of a week working on a deliverable only for it to get a review explaining why it won’t work / could be done better and had to re-write most of my code.
How to write Smaller PRs
“But I can't!” I hear you cry.” This deliverable needs this much code, my PR needs to be this big!”. Just because a feature, bug fix, or deliverable requires lots of code doesn’t mean you need to wack it all in one big PR.
Codeplans
One strategy that I've used before when working on a task that will take a significant amount of work is to write a code plan outlining what I’m planning to do which I’ve shown to my teammates. These plans typically include interface definitions, class outlines, and the broad strokes of implementation details. This allows me to get opinions on the entirety of my solution without having all the code together.
‘Not Implemented Yet’
Most programming languages have a specific type of exception called something like ‘NotImplementedException’. When you are writing a class that has to meet a particular interface you can typically use a shortcut in the IDE to add all the required methods with a method body that just throws this exception. Now your implementation will compile and have its desired interface without you actually having to add any logic code.
When I need to add a complicated class to a codebase I will typically follow this pattern and fill in the methods gradually, in steps that are digestible.
Duplication isn't always bad
Sometimes you may find yourself working on a task to replace something in a system with something else. Website page, Interface implementation, it doesn’t matter what you're replacing. As programmers, we are taught to avoid duplication so the temptation is to replace A with B in one fell swoop. However, it might be better to have A and B coexist and built B up gradually until it reaches parity with A.
If A and B are code you could gradually replace calls to A with calls to B as it is written. If A and B were UI components you could use a feature flag to ‘turn on’ B whilst leaving A as the default.
Don’t Just Trust Me
The concept of Continuous Integration is described as “a DevOps software development practice where developers regularly merge their code changes into a central repository, after which automated builds and tests are run”.
Regularly here can mean multiple times a day! For most work, this is too frequent to be completing work items. Instead, the strategies outlined here are employed to allow developers to stay productive.
On the internet, you’ll be able to find lots of well-respected engineers advocating on CI as a way for teams to stay Agile, write robust code and build things quickly. Here are a couple of links if you want to know more:
If you enjoyed this article you might like my other article Should we be deploying every day?
You may also like my TikTok joetalkscomputing.
