Code reviews are a vital part of the modern software development life-cycle. Tools like Github, Gitlab, and others have made the code review process easier to facilitate than ever before. Online static analysis tools like Codacy, LGTM, and others can provide additional feedback right in a pull request with just some configuration. Many companies – including where I currently work, Bandwidth – even require a minimum number of reviewers to approve before merging to the mainline.
But code reviews can often be a huge point of contention, and often for the silliest of reasons (which I will get to later). They also can drag on longer than needed, slowing your team’s overall process.
So how can you become a great code reviewer without getting bogged down in the weeds and keep your process efficient? Below are some of my recommendations for doing just that.
Learn to LOVE Reading Code
I don’t know about you, but I love reading code. I’m not kidding or exaggerating – sometimes I wish I was. When I get an email to review another pull request, I get super excited to spend an hour or so reading someone else’s changes.
It didn’t come naturally at first. I would have rather written my own code instead of reading others’ for the first few years of my career and in college. In my first internship, I decided to rewrite some code from scratch than learn to understand the code from the previous intern. I learned rather quickly, however, that this is unsustainable. You will need to learn to love to read code if you want to be a successful software engineer.
One of the good things about learning to read code is how much it will help you learn other languages and tools. If you can become good at reading code, then you will be able to learn the patterns of any language quickly, which is helpful in the long run.
Agree on the “Small Things”
You need to agree on the small things before any code even gets into code review. These include:
I’ve seen more time wasted on code style in a PR than any other. Get together and pick a style (they are plenty out there already to use as starts), add a linter, and move on.
This could be rolled up into Code Style as well, but I’ve always seen these two ideas as separate. Either way, agree on the naming for your classes, variables, methods, etc. Especially pay attention to things like acronyms or abbreviations (for example, whether to use
Tests and Code Coverage
Make sure everyone understands the level of testing expected for your project. It’s a terrible feeling to spend time working really hard on a story, submit a PR, and be told you need to add tests before you can move forward (which should happen b.t.w). If your team is doing TDD, this should really be a non-issue.
Along with testing, agree on code coverage beforehand. Set a threshold using the various tools online for tracking coverage and fail the build it if falls below a set threshold. If I’m assigned that PR, I can treat the lack of coverage as a lack of tests and ignore the PR until the build passes.
Make sure your team agreed upon the requirements before it was even picked up. This should go without saying, but its a crucial and overlooked step. As a reviewer, make sure to re-read the story before reviewing code instead of making assumptions. As I get into in the section below, code reviews are about understanding. That also means you need to understand what the story is doing vs what the story is requiring it to do.
Code Reviews Aren’t About Finding Bugs
There are articles and articles that show that code reviews are effective means to detect and triage bugs before release. However, I would argue that isn’t the point of a code review. Instead, I believe that finding bugs in someone’s code is really a side-effect of a good code review; not that other way around.
While Steve McConnel made a huge stride in pushing for code reviews in his seminal book Code Complete, his whole angle was that code reviews mitigate bugs. And he is right. But I think the reason code reviews have such an effective detection rate is because a good reviewer is trying to understand the what, why, and how of a code change.
As a developer, if I can effectively find the answers to those three questions with confidence and clarity, then likely the implementation is correct. If I don’t understand what the changes actually are (because of a reformat or small changes in lots of places) then I ask the reviewer to explain it in further detail. This might expose a change they missed. If a variable name feels off or its scope is too large, I’ll ask why, which might expose that the variable indeed has too large of a scope and was being updated where it shouldn’t be. If I don’t understand how the implementation achieves the requirements of the story, then I will ask how, which might expose the requirements weren’t understood and the story needs to be reworked.
The idea here is that instead of focusing on fiding bugs or pointing out what is wrong with a set of code changes, focus on understanding those changes. For even more on this idea, see my article Whats the Points of Code Reviews Anyway.
There is a lot more out there on how to become a great code reviewer (see a list of some of my favorites below). I’m a firm believer that its a vital part of the software development process and should be embraced rather than shrugged off.
How have you learned to become a better code reviewer? Let me know in the comments!