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 the their part. Spend some time advocating in your mind for the code you’re reading, challenging your own assumptions. Always start by acknowledging the thought and work that the author put into the code, before giving them any further feedback. If you understand where the author is coming from, take time to acknowledge it before providing counterarguments.
2. Objectivity > Subjectivity
Seek out objectivity in all arguments
A comment asking for a change should make an objective, respectful 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. The only acceptable kind of subjectivity is when the code appears confusing. In such a case, ask if we can find a way to make the code easier to follow. A healthy team always strives to address confusing code.
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. Ask, Don’t Tell
Turn statements into non-leading questions
Every time you are making a statement, try to turn it into a non-leading question. Let’s say you think code should be moved to another function.
- The original thought: “This code should be moved to function X.”
- The question form: “Could you move this code to function X?”
- The non-leading question form: “What are your thoughts on moving this code to function X?”
In the end, be genuinely willing to embrace and accept counterarguments, or open up a healthy/constructive discussion.
5. Care About Details
It is not a waste of time to discuss a detail in depth
We have a term in our community: “bikeshedding”. Let’s unlearn this term in code reviews. Details can matter because technical debt tends to be a “death by a thousand cuts”. Besides, a big 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 cause more damage than actually staying on topic. The fact that 2 intelligent people are genuinely interested in finding a good outcome is enough of a justification for having a discussion, and you shouldn’t be worried as long as it stays constructive.
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 thoroughly articulate your thoughts to increase the likelihood of the other person being able to follow your comment. This is especially important when collaborating across time zones. Keep in mind that despite your best efforts they might still not follow it, and this is ok, we are not trying to rush through this. If articulating your thoughts is extra difficult, feel free to collaborate on a problem by writing up and submitting a pull request with your suggestions into the branch that you’re reviewing. Be willing to agree not to merge it, since this is just a part of having a discussion.
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 a fruit of the labor of its author. 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, try to approach the reviewer with empathy and appreciation for the ideas they’ve put forward. It never hurts to let people see that you appreciate their work.
9. Use Complete Thoughts
Fight the instinct to leave a quick one-liner
One-liners are notoriously bad when it comes to unintended signaling. Here are some of the ways they can be misinterpreted:
- my work is not worthy of your proper consideration
- you don’t care whether I agree or disagree
- you don’t take code reviews seriously
- your time (writing) is more valuable than my time (unpacking what you mean)
- you’re rubbing my nose in how immediately obvious the issue was to you
For tiny issues, writing a complete sentence might feel like an overkill. However, regardless of topic, a well written comment is never a waste of time, since it’s meant for a human to read. Even if you think you know the recipient well, anyone can have a bad day and take things close to heart. They may not show you any signs, since nobody likes to come off difficult, but the overall perception can wear one down over time. The onus is on reviewer to keep it in mind.
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!
P.S. Emojis, gifs, memes, and puns are welcome and encouraged.
Special thanks to the awesome Philip Szalwinski for suggestions and contributions.
Top comments (0)