Code Reviews are First-Class Citizens
Code reviews are an integral part of our daily work as engineers. They help us reduce bugs, share knowledge, collaborate asynchronously, build rapport, feel recognized, and most importantly, keep software maintainable. Diligent code reviews can save the team from insidious architectural mistakes that may hinder all future development. So why do we often treat them as second-rate citizens, a distraction in the way of shipping? Why is it wrong for a good day of work to consist entirely of leaving PR feedback? Why do we try to sneak reviews past stakeholders, and outright skip them in the face of movable deadlines? There are definitely reasons for it, but whatever they are, it might help to take a deeper look at your engineering culture. I believe that in a growing (and especially geographically distributed) company, engineering success is predicated on embracing code reviews as first-class citizens, with full stakeholder buy-in.
Writing Under Pressure 
To members of the computer generation it’s no surprise that it is easy to accidentally come off dry, dismissive, judgmental, or worse in text. This is exacerbated by constantly putting out fires and missing deadlines while trying to leave feedback. On the receiving end, people most likely take pride in their work, and are attuned to listen carefully. Reading PR comments then becomes sort of like putting your ear directly onto an active megaphone. This is why I believe that rushed code reviews are harmful to engineering culture. All this pressure makes being kind and considerate a more difficult challenge than it has to be. If your company is a burning tornado, fixing that could be a crucial step towards mindful… everything, let alone code reviews.
Practices
Over the years in the industry I’ve compiled a list of my favorite code review practices. Here they are in no particular order.
1. Advocate for the Reviewee
Approach each comment from the position of respect for author’s work and decisions
Even when some of the author’s decisions appear to be “clearly suboptimal”, or straight up mistakes, assume the best intentions on their part. Spend some time advocating in your mind for the code you’re reading, challenging your own assumptions. If you understand where the author is coming from, acknowledge it before providing counterarguments.
2. Objectivity > Subjectivity
Seek out objectivity in all arguments
A comment asking for a change should make an objective case for it. When making a case, dig past personal preferences all the way down to objective underpinnings of your argument. A tiny nugget of strict objectivity is miles more effective than a 500-word opinion piece.
There’s one good kind of subjective comment: the code confuses you. Since the most important measure of maintainability is that code is clear for people on your team, “confusing” comment gets a special pass.
3. Conversation > Silence
Subjectivity is welcome as long as it’s a discussion
Sometimes we can’t help but voice a subjective opinion. In doing so, we must acknowledge that we have been unsuccessful in finding an objective argument, and are asking the author to indulge us for a moment. This is ok, as long as we present these opinions as topics for discussion, and not as something we insist on being implemented. Use the discussion as a tool to figure out the objective underpinnings behind your opinion. The team should support this exploration, and try to learn from it. Of course, be willing to accept that the discussion will not always result in your opinion making its way into the code.
4. Assume Competence
Use question form when suggesting something seemingly obvious
When you are suggesting something that appears obvious to you, it’s possible that you’re missing a problem that the author may have already discovered. If you don’t invite an explanation, the author may feel compelled to make the requested change, and work around the problem in some other way.
Instead, switch your statement into a genuine question. Let’s say you think code should be moved to another function.
- The original thought: “This code should be moved to function X due to [reasons].”
- The fake question form (don’t do this): “Could you move this code to function X due to [reasons]?”
- The genuine question form: “Have you considered moving this code to function X to avoid [reasons]?”
Notice how we are still being concise, and are still providing our solution. Except, now the author gets to choose. They can either explain why they did what they did (and maybe you’ll end up agreeing), or they can follow the request without wasting another round.
P.S. In my experience, this is the most powerful “hack” in this whole list. It’s incredibly easy to switch to a question form, not obscure any valuable info, and yet completely remove any sense of bitter judgement from your comment.
5. Care About Details
It is not a waste of time to discuss a detail in depth
Details can matter because technical debt tends to be a “death by a thousand cuts”. Besides, a discussion over a small detail can often be useful for other things, like establishing a rapport with someone. A self-conscious fear of wasting time could end up wasting more time than actually staying on topic.
6. Specific Examples > Generalizations
Try to propose a concrete solution
If possible, use pseudo-code or real code (untested is ok) to illustrate your points. If writing the code is not feasible, take time to make your comment easy to follow. This is especially important when collaborating across time zones.
7. Working Code > No Code
Always respect working code
If a fellow engineer submits a PR with a working and tested implementation, but you find that it could use a better architectural approach, this is a great problem to have. Now we can focus on refactoring this PR without worrying about implementation details, since they are already working and tested. This actually frees us to collaborate on reshaping the code’s architecture while maintaining the same logic.
8. Advocate for the Reviewer
A code review itself is an original work
When you are on the receiving end of a code review, treat the review itself as its author’s work. Even though they’re reviewing your code, their review is their original work. Instead of only focusing on the changes you’ve been asked to make, express some appreciation for comments that you found useful, or their effort to understand your code.
9. Use Complete Thoughts
Fight the instinct to leave a quick one-liner
It’s okay to use one-liners in a considerate way (i.e. as per point 4). However, if your one-liner is a short and dry change instruction, you are sending some bad signals, like:
- I don’t care whether you agree or disagree
- I don’t see you as my peer
- I don’t take code reviews (or reviewing your code) seriously
- My time (writing) is worth more than your time (unpacking what I mean)
- Your mistake was obvious to me
Practices in this article will help you avoid sending these signals.
10. Practice
These aren’t rules to be followed perfectly from day one
These practices aren’t meant to be a checklist. As long as you follow these practices in spirit, it’s ok to make your own judgment calls based on specific situations. The more you practice, the easier it gets.
11. Have fun!
Enjoy geeking out on technical discussions with your colleagues
Code reviews are places where we get to unapologetically talk deep programming, so let’s take advantage of it, and have fun!
Special thanks to the awesome Philip Szalwinski for suggestions and contributions.