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 will not break anything? All of these? Non of these?
All these questions are necessary parts of the code review, but none of them is its ultimate goal.
The primary purpose of the code review is to make sure that the solution is solving the problem in the best possible way. From the code and product perspective.
All of the code review practices are designed to this end goal.
But how do you determine if the solution you are reviewing is the best way to solve the problem? In other words, how to code review?
Let’s deep dive into it.
No “perfect” code
Before you start reviewing code you need to understand that there is no such thing as “perfect” code – there is only a “balanced” code.
You as a reviewer should balance out the need to make forward progress compared to 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 but it’s not “perfect,” don’t delay it for weeks. Leave a comment on how the author of the changes can improve his code, and give it a green light.
Your opinions are not authority
Everyone has their personal opinion on everything: how you should write code, how many hours, 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 are deciding if the solution you are reviewing is written “good” or “not”, you have to rely on technical facts and standards, not personal preference.
For example, while coding, you always use a semicolon at the end of the line, but the author of the code you are 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.
Codebase must be consistent and not written by personal preferences.
I have 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 that he was wrong, still decided to bend his line just not to look stupid that he didn't know something.
Code review is not a ring where you need to dominate your opponent. You are one team. You work toward the common goal. Everybody is your teammates.
The first thing you should do in the conflict is to come to a consensus. Better to do it not in the comments on GitHub but in chat or video call, it is much faster.
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 it is needed in the task.
- Changes are not 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. Not long and hard to read.
- **Author added comments where necessary. **Good comments are those that explain why some code exists, and not what 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.) the author needs to create or update 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 he is not already.
Make sure that tests are not written to test themselves. They must be useful and valid.
First of all, abstract from your feelings about the person whose code you are reviewing. Always make comments about the code and not a person.
If you see that the author wrote a “bad” code and it somehow triggers your emotions like anger, calm down, and leave your comments on a calm mind.
Also, never ever write comments like “do this and not this”, always explain why the author should make changes and add the 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 can make their day.
In the End…
If you like this article share it with your friends and check me on Twitter.
📌 Every week, I send out a “3-2-1” newsletter with 3 tech news, 2 articles, and 1 advice for you. Join my 3-2-1 newsletter here.
Top comments (3)
It's important to keep a friendly and safe environment. There should be no fear of failure, no blame, etc. In fact, coding failures should be celebrated, since everyone learns from the mistakes.
Thanks Edwin, appreciate that!
You are right, in such environment we can grow in a healthy way.
Hope this help..