5 Ground Rules For Better Code Review
How many times did you lose yourself as a reviewer, while going over a messy code, with an author who constantly has to explain what he did, only to find out the code won’t even compile? And how many times, did you feel, as an author, that the code review was meaningless, and it was just merged away instantly, with no lessons learned?
If you relate to any of the above, or similar, here are 5 ground rules I highly recommend you follow, both as an author and reviewer, that will greatly improve your code review process and help you create a much greater and long-lasting impact when doing so.
- Avoid Pull Requests, do it Face to Face.
Sure, pull requests save time, it’s organized and easy, but think about it for a minute — the knowledge transfer act that is just being shoved into a small github textbox, with no conversation or basic human interaction, with a chance for the author to properly explain himself.
Is coding as simple as black and white? Wouldn’t you rather actually learn how to write better code, both when reviewing or writing?
Face to face code reviews create conversation, a vivid exchange of ideas. It lets the reviewer offer suggestions and tips that won’t necessarily change your code, but will open your mind the next time you write something similar. And also gives the author a chance to explain why he thinks that what he did is better, and who knows, maybe the reviewer will also learn something new.
2. Reviewer takes the wheel, while the author sits and watch.
Face to face code reviews do have a major drawback. Having the author explaining his code throughout the whole review, might make it difficult to understand if the code is clean, and at all readable? Other programmers will have to deal with his code, without him sitting next to them and explaining his writings step by step — and that’s where this next rule comes in.
The reviewer should handle the code alone, and the author should only answer when approached, the author doesn’t initiate a conversation, he only replies when addressed.
This way the conversation about code readability is honest, and the reviewer isn’t mislead. I know it sounds pretty strict, but it works, and you can still be polite about it.
3. The Author is “Responsible”.
Don’t waste the reviewer’s time, make sure that your code compiles, all your tests are green, and take a final quick look at your code (avoid typo, lint issues, etc) before moving on to the review stage.
We don’t want the code review to be about technicalities, remember, it needs to be meaningful.
4. Never Say “You”, Say “We” Instead.
You’ll notice that if you use the term “You” too much, the author might get defensive as he protects his code (and maybe feels his dignity is on the line as well). Be sure this code review won’t be productive and not at all meaningful.
Refer your argument to the team or yourself, it’s nicer and opens up the author to be more attentive to changes.
For example: “What you did is wrong” => “What we usually do is that” / “I usually do it like that, what do you think?”.
Yes, that kind of “you” is ok :)
5. Let the author choose his reviewer.
This tip is aimed at the managers amongst us, who assign the same reviewer (mostly themselves) to do code reviews for their team. In my humble opinion, when it comes to code reviews, experience should have little meaning, and let me blow your mind: A senior can learn something from juniors too!
When there is only one reviewer — he just makes replicas of his own opinions and coding style on the entire team, instead of having a “mix & match” of the opinions and coding styles the entire team agrees on.
Summing up, I’d like to leave you with one note:
Code reviews should be about knowledge sharing and self enhancement. A better codebase is just a bonus, and it will come when your team members respect and learn from one another.
For more updates and insights about Software Engineering — follow me on Twitter
