The Complete Guide on How to Code Review
Best practices from Apple, Google, Microsoft, Amazon, and Facebook
What’s the end goal of the code review?
To make sure that changes work? To validate if code is following the style guide? To check if new changes won’t break anything? All of these? None of these?
All of these questions are necessary parts of the code review, but none of them are its ultimate goal.
The primary purpose of the code review is to make sure the solution is solving the problem in the best possible way — from both the code and product perspective.
All code-review practices are designed to meet this end goal.
But how do you determine if the solution you’re reviewing is the best way to solve the problem? In other words, how do you code review?
Let’s deep dive into it.
There’s No Perfect Code
Before you start reviewing code, you need to understand that there’s no such thing as perfect code — there’s only balanced code.
You, as a reviewer, should balance out the need to make forward progress with understanding the importance of the changes.
Stop seeking perfection.
You don’t just write code for code — you solve a business problem with the code.
If code solves a problem and improves the maintainability, readability, and understandability of the system, even if it’s not perfect, don’t delay it for weeks. Leave a comment on how the author of the changes can improve their code, and give it a green light.
Your Opinions Aren’t Authority
Everyone has their own personal opinion on everything: how you should write code, how many hours you should spend on it, why you need to use some text editor and not another, and many others.
We all have different experiences and different backgrounds, but we need to turn them off during the code review.
When you’re deciding if the solution you’re reviewing is written well or not, you have to rely on technical facts and standards, not personal preference.
For example, while coding, maybe you always use a semicolon at the end of the line, but the author of the code you’re reviewing doesn’t. The first thing you want to do is leave a comment like “Add semicolon here.” But what you should do instead is create a style guide and make it a standard in the team.
The code base must be consistent and not written to conform to personal preferences.
I’ve seen a lot of examples where the reviewer was wrong about “how to write code,” left a comment about it while code reviewing, and after realizing they were wrong, still deciding to bend their line just to not look stupid or like they didn’t know something.
Code review isn’t a ring where you need to dominate your opponent. You’re one team. You work toward the common goal. Everybody is your teammate.
The first thing you should do in the conflict is to come to a consensus. It’s better to do in a chat or video call, not in the comments on GitHub. It’s much faster in the end.
If both of you can’t find a common solution on how to write code, ask your team or technical lead for help.
Don’t sit down on one conflict for days — try to resolve it as quickly as possible.
On a high level, you need to look at two things while code reviewing:
- Changes solve the problem.
- Code is well designed.
To do this, create a checklist of what you should pay attention to when reviewing the code, and use it on every code review.
Here are my general recommendations on what you should look at:
- Changes do what the developer intended. Everything works as needed in the task.
- Changes aren’t too complex to understand. All members of the team can understand every line of code and can easily maintain this code in the future.
- Author picked good names for everything. They’re not long and hard to read.
- Author added comments where necessary. Good comments are those that explain why a piece of code exists — and aren’t just explaining what the code is doing. If the code isn’t clear enough to explain itself, then the author should make the code simpler.
- Changes follow the style guide. If you don’t have a style guide in the company, you should definitely create one.
- Do changes need documentation? If they change the main part of basic or main functionality (build, test, API, integration, etc.), then the author needs to create or update the documentation about it.
Every change must be tested.
Ask the author to add unit, integration, or end-to-end tests as appropriate for the change, if they haven’t done so already.
Make sure tests aren’t written to test themselves. They must be useful and valid.
First of all, abstract from your feelings about the person whose code you’re reviewing. Always make comments about the code and not a person.
If you see the author wrote “bad” code and it somehow triggers anger, calm down first, and then come back and leave your comments with a calm mind.
Also, never ever write comments like “do this and not this.” Always explain why the author should make changes and add a source (link to the blog post, documentation, guidelines) where the person could read more about it. But don’t write a complete solution to the problem when you explain why. Code review must not take you the whole day. And it’s not your job to fix the changes of another developer. Find a balance between pointing out the problem and providing a direct solution.
Be a Human
And the last one: Be a human.
If you found something nice in the changes, tell the author about it. Code reviews should not be just about mistakes. Give some positive feedback as well. Be a mentor.
For you, it may be just a small positive comment, but for someone else, it could make their day.