Developers should look forward to code reviews, not dread them

Code reviews are a great place to exchange knowledge. Developers have the opportunity to learn from their peers, to showcase a piece of code they’re particularly proud of, to share what they discover to be a better approach to a known problem, and to leverage the expertise of teammates. Ideally, a working piece of code becomes better through the ensuing dialogue between the author and a reviewer. That’s the goal, that’s the purpose of code reviews, to foster learning, encourage feedback, positive reinforcement and add experiences to a whole that is greater than the sum of its parts.

I enjoy code reviews — as an author and as a reviewer. Once I’m done with a piece of code and have that feeling of a job well done that goes with completing a task, I actively seek my peers and ask them to review my code, because I hope they’ll point out something I missed and I’ll learn something, or they’ll see something they don’t know and try to learn from it, or they’ll just say “hey that’s cool, I like the way you’ve done that”. Sometimes, they save me from an embarrassing bug. Whatever it may be, I’m looking forward to what they have to say.

So why are they too often a source of fear and existential dread? Why are there plenty of code review horror stories? Even Linus Torvalds has become famous for his tirades and rants, and had to take a break after recognizing that the way he treated some other developers was not right (coincidently, I’ve socialized a few times with the developer on the receiving end of this brutal review, and when I reached out to offer moral support, he defended Linus and justified his behavior, much to my surprise, which leads me to assume that this behavior is deemed acceptable in certain ways). If some developers have come to dread code reviews, it’s because of… other developers. It’s very odd that the same people who dislike code reviews are the ones responsible for them, this would seem like an easy problem to fix, so what’s wrong?

I started asking myself why code reviews are something I’ve come to look forward to when many people dread them, and I’ve tried to come up with ideas on how to make them a positive experience.

Don’t be a jerk

Developers need to submit code free of preventable mistakes

To avoid unhelpful nitpicking, the team should have:

  • A properly configured linter, supplemented with custom rules if necessary
  • An agreed upon style guide. Defer to an established standard like Google, AirBnb, Sun/Oracle to avoid battles of opinions
  • A common formatting configuration for IDEs (like editorconfig)
  • An automated code formatter using a git hook (like husky/lint-stage/prettier)
  • Documentation for known anti patterns
  • Documentation detailing what to look for in a code review. (e.g: scope, correctness, design and style, in that order. This is from CS50)
  • Unit tests with adequate coverage

Document this under a team agreement that is available to all team members. If one of those choices comes under attack, it becomes a whole lot easier to say “this is detailed in the team agreement” rather than “I like my way of doing things better”.

Small common mistakes are easy to pick up and to fret over. They’re common because they’re easy to make. Many tools exist to avoid these common missteps and can significantly bring up code quality. By using tools that prevent them from finding their way in the code, reviews can focus on substance over style.

Reviewers should be helpful and meaningful

  • In an international context, use simple global English, free of idioms and cultural references.
  • When possible, reference official documentation and established best practices.
  • Emojis are useful to convey context in the absence of body language.
  • Comments should be meaningful. I once left a comment that was just one line of code in a pull request. This degenerated in a full blown debate on whether to use map or reduce. After about half a dozen comments from my team mates, I ended the conversation by explaining that the point of my comment was to show the developer that their for loop was not necessary and could be replaced by a call to an Array method. I obviously failed to make my point. My comment should have been “You can shorten this code by using reduce” with a link to the MDN documentation.
  • Comments are not meant to be conversations. Use your phone/slack/corporate instant messenger/email. If you’re in the same office, walk over to that person’s desk. Value human interactions.
  • Remember the golden rule, don’t be a jerk, don’t be condescending, don’t be pedantic and don’t be patronizing.
  • Learn how to give and receive feedback: assume positive intent, be clear, be helpful, think about a time where you may have given/received similar feedback, etc…
  • Comments can be used to give praise!

Finally, lead/senior developers need to earn their title and remember that they’re meant to be mentors, and they need to lead by example. When a pull request is way off, leaders need to ask themselves if they did everything that they were supposed to do: offer proper guidance, make sure the requirement was understood, the scope of the work clearly defined, and that the task is within the skill area of the developer, regardless of level. If the author of a particularly wrong pull request is a junior developer, someone needs to sit with that person. Start the conversation with “I was reviewing your code, and there are a few things I didn’t understand”, or “hey, I checked out your branch, and I think we can improve this, let’s look at it together”. Junior developers should be in an environment where they feel safe to ask for that if it’s not happening.

In the end, we developers are responsible for conducting code reviews, and it is up to us to make them what they should be, an enriching conversation.

Previously Paris and Geneva, currently New York. Can also be found at https://dev.to/arnaud