How To Avoid Making Giant Pull Requests
It can be difficult to carve your pull requests into manageable chunks. But a good developer will consider the code reviewer’s process. As part of our code quality process, every bit of code that goes into one of our applications is reviewed by a second engineer or designer (i.e. not the author). We do this using GitHub pull requests and “diffs.”
We try to keep the changes in each pull request small. Each pull request should be no more than 300-400 lines changed (and ideally < 100 lines changed). This is because small pull requests are much easier for a reviewer to grok. When a reviewer groks the code, they can find more bugs, better manage risk, and give specific feedback.
Engineers can find it challenging to build major features while still making small pull requests (PRs). Here are some tips on how to make your pull requests smaller and easier to review:
Know this: you can make as many Pull Requests as you want.
No one said that one PR = one feature. You might need more than one PR to express your feature clearly, but can’t afford to ship the code in small pieces. Make a feature branch!
Make as many branches off of that feature branch as you want. Get your code review when you merge from working branches into the feature branch. Then, when you get ready to merge the feature into master or develop, it has already been reviewed (as it enters the feature branch).
Good PRs adhere to a single responsibility.
A major reason why big PRs are hard to review is because they change a bunch of things that are only loosely related. Be honest with yourself: can you not break down this change into a smaller unit and still have it stand alone?
When we apply the single responsibility principle in software design we can fool ourselves into thinking an object has only one reason to change, when it has many. In writing and reviewing thousands of PRs in my career, I’ve learned that we can just as easily convince ourselves that a grab-bag PR is a single change.
Be skeptical. Slow down.
When you are developing a big feature, you must force yourself to slow down and proceed deliberately. Don’t only think about what to change in the code to build a feature. Consider how to present your work to your reviewer. Consider this checklist to separate out PRs that are de-linting and style cleanups, or dependency upgrades and other changes.
Break your change apart.
Look for anything you can carve away from the “big change.” Consider:
- What can you do first that lays the groundwork for this feature?
- Can you make backward-compatible changes first in some separate PRs?
- Can you make database changes before changing UI?
- Should you clean up all your “lints” in one pull request before you even start your refactor? What about your other “clean-ups”?
- How about you start by just beefing up your test coverage and get a separate review before you start refactoring any business logic?
Chip away at the differences.
It can be good to introduce temporary (or permanent) shims, facades, or adapters. If I’m combining two classes in a refactor, I’ll start by making Class B inherit from Class A and kill any code in B that is the same as A. This leaves me with two working classes and less code than before. I’ll commit this change and get it reviewed.
When breaking classes apart, I’ll do something similar by making my new Class B proxy through to Class A, before I even introduce any unique code in B. Commit and review. Eventually, there’s nothing B shares with A and I break the link, but in the meanwhile, I can chip away at it, one method at a time. Commit and review.
Find the easy way.
Can you make your search-and-replace rename a separate PR? I love getting those PRs to review. They are so easy.
You must watch the size of your diff locally. You can use
git diff, the GitHub desktop app, or the GitHub compare view on the web to check the size of your diff and know when you are biting off too much.
Consider all the tools at your disposal.
There’s not a single approach that will improve every single pull request. It’s a tool belt, and you have to grab the right tool for your situation. However, with careful work and enough techniques you can build better pull requests and get better code reviews.