Developers should look forward to code reviews, not dread them

Don’t be a jerk

My teammates are not jerks, and it’s not only about who they are as individuals, it’s also about who we, as a team, decide to be. My manager wouldn’t tolerate me being a jerk on pull requests, and I wouldn’t tolerate that from my team members either. So rule #1 is the golden rule: treat others the same way that you would like to be treated. That one’s not hard (and it’s applicable everywhere). It starts from the top, and it’s taught by example. Sure, I’ve had bursts of anger where I see the same mistake made by the same developer over and over. A negative reaction would be to lash out, “how many times do I have to to tell you not to do this”. A more positive and constructive reaction is to simply ask: “hey, I see you do this over and over, what is it about this pattern that’s so appealing?” without emitting judgement either way, and instead of conflict, there is dialogue.

Developers need to submit code free of preventable mistakes

When reviewing code, there’s a natural tendency to want to leave a comment, any comment. Even if there’s nothing wrong with the code, there’s probably a curly brace that’s not aligned, or a typo somewhere. After all, if someone is tagged as a reviewer and doesn’t leave a comment and just approves, they’re not doing their job, right? Wrong! A junior developer recently shared their experience interning at a large social media company. They fell in a slump because their PRs kept getting picked apart with comments that add little value: “remove extra space here” or “rename this variable”. Every comment and associated correction would send an email to a bunch of other people, adding more scrutiny and pressure, until the developer came to feel that no code contribution would ever be good enough and stopped opening pull requests. That should have raised all kinds of alarms to the lead developer/team manager, but instead the developer was reprimanded for “falling behind” and not opening enough PRs. It’s easy to see how this became a toxic environment.

  • 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

Reviewers should be helpful and meaningful

If reviewers are not looking at code that’s poorly formatted or breaking standards, they can now focus on more substantive quality criteria, like scope and correctness. Hastily written comments can lead to confusion which leads to frustration. It is the role of reviewers to be easy to understand when leaving comments.

  • 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!

--

--

Get the Medium app

A button that says 'Download on the App Store', and if clicked it will lead you to the iOS App store
A button that says 'Get it on, Google Play', and if clicked it will lead you to the Google Play store