A Code Review Your Colleagues Would Thank You For
“You’re a code review Terminator” — once told me a colleague in response to an especially productive code review. It was a win-win: he learned something useful, and I enjoyed knowing that the review I’ve done allowed my peer to become slightly better himself.
It wasn’t a breeze, though: after submitting review I left the desk for a break, completely exhausted. Was it worth it? Hell yeah!
I wanted others to feel that joy of being thanked for a decent review. So I analyzed my approach to code reviews and broke down core elements item by item.
1. Treat reviews as one of your main activities
Leaving a couple of “a null check here is missing” comments over a cup of coffee is not enough. You need to:
- understand what should have been done
- understand how it was done
- think of other ways it could have been done. Are there better (more readable, maintainable, efficient) ones?
- if the current approach is optimal, verify it is implemented correctly. Otherwise suggest another and explain why it might be better.
A high-quality codebase is possible only if the majority of the changes is high-quality. Write good code — and help others do the same. Constantly.
2. Compensate lack of nonverbal signals
Reviewing code sitting right next to your mate is one thing; doing it on platforms like Github or Bitbucket is another. When you don’t get additional clues from voice tone and body language of your peer, it’s easy to misinterpret original intentions.
Let’s take a simple phrase as an example: “There’s a bug in this line”. It surely states that, well, there is a bug in line. But it could have been said with very different emotions attached: anger, surprise, joy of finding a bug and thus preventing an issue on production. Indifference, maybe. Your colleague might make a wrong assumption — and wrong assumptions lead to offences, hidden tensions and in general are not good. Just make your life easier and avoid the issue altogether: soften a phrase a bit (“Hey, looks like there’s a bug here”), make it a question (“It seems suspicious; is it intended here?”), or add an emoji, maybe 🙂.
3. Reserve time
To ensure code is correct and efficient you need to understand it completely. However, it takes time; make sure you allocate enough. Keep in mind that reviewing parts of the application you don’t know will take even longer.
In general, it’s the flip side of good reviews: it’s one pretty time-consuming activity. If you don’t have enough time to do to it well — consider to postpone it or to ask to assign another person. If that’s not an option and you have to do just a shallow review — keep in mind that you’re trading off quality. Probably necessary, but a trade-off anyway. Making it a habit might mean more technical debt to fix in the long run.
4. Don’t assume; ask
When you see something strange in the code or maybe an overly complex approach to a seemingly simple issue — don’t assume that they made a poor choice or a mistake. There’s a chance your colleague did that because of reasons you aren’t aware of. Avoid awkwardness and hidden tensions, ask instead of making assumptions: “Could you please clarify why we can’t call this method directly here? It seems like a simpler approach which could suffice in this case”.
5. Know when to reach out directly
Usually, reviews are done in an async manner. This allows you to immerse yourself into the code and at the same time avoid interrupting the flow of your peer. However, there are several situations when it might be better to reach out to your colleague by either walking by or having a call:
- When deadline pressures. Quick feedback speeds up decisions. However, people are already stressed out and distractions could cause irritation and lose of focus.
- Gross errors. Discussing them publicly might cause embarrassment. Better to talk to a person privately. It might have been an overlook — or a knowledge gap which is now eliminated with your help.
- A completely wrong approach chosen. Communicating that one should throw away part of his job should be done with care, and it’s better to use the power of body language and voice tone to do that.
Frequently it is useful to provide a summary of such conversation as a comment in your review system afterwards.
6. Read ticket first
Some of the requirements might be not covered in an otherwise correct pull request. Ensure you read the ticket first to catch such issues — and to understand what’s going on, at least.
7. Explain the reasoning behind your suggestions
Some mistakes, such as typos, don’t require additional explanations. However, architectural and naming decisions is a complex matter. When you suggest an alternative approach, it might be not obvious for another person why exactly it is better. So, accompany your suggestion with a brief list of its benefits and downsides.
Also, as a proverb goes, “give a man a fish and you feed him for a day; teach a man to fish and you feed him for a lifetime”. When you teach your peer a new trick, (s)he would be able to apply it in future to produce better code — with an increase of overall codebase quality as a nice side-effect.
8. Compliment good decisions
Code review doesn’t have to be a mistakes list. If you stumble upon a good place, nice decision, useful piece — communicate your appreciation. Leaving a “good job” comment or a clapping emoji as an overall review response might be worthwhile, too. Just be sure you don’t do that for actually bad pull requests — that would feel mechanical and ruin the whole idea.
9. Be polite
A hint: a phrase like “Can we please get rid of the brain-damaged stupid networking comment syntax style?” isn’t exactly polite.
10. Be helpful
For example, when during review you spent some time to find an alternative decision — provide your colleague with names of classes you’ve checked. Github and other systems make it even easier by allowing you to give a link to the exact line in a file. Acting in a way like this requires little additional effort but helps your colleagues and increases their performance.
11. Suggest, don’t tell
This goes alongside “be polite” advice. When you suggest an alternative approach during code review, provide your argumentation and let your colleague decide instead of telling what to do. Chances are (s)he would accept your advice. If that’s not the case — what was the reason?
- Both approaches are more or less equal. If there are no compelling reasons to choose one way over another — keeping existing is fine.
- There’s a compelling reason to use your approach but apparently, PR’s author doesn’t understand it. You could try to explain one more time — and check one more time that you’re not mistaken.
- Personal conflicts. This is a subtle matter and should be handled cautiously. It’s outside of code review topic for sure.
And that’s it for the list. To wrap up:
Make the world around you slightly better. Do good code reviews.
Interested in other aspects of code reviews? I’m working on another article, “Help people review your code”; so stay tuned!