Better Code Review: Part 3

Reviewing code

Happy new year! The beginning of the year is a great time to reflect on our strengths, our accomplishments, and our areas of opportunity. Perhaps you’ve decided that code reviews are one of your areas of opportunity. Wonderful! I’m so glad you’re here. I highly recommend checking out parts 1 and 2 of this series, as code review is a two-way street. Now that you’re caught up, let’s move onto part 3: reviewing others’ code. What follows are a few tips and tricks that I’ve learned and (tried to) put into practice myself.

Language matters

Being condescending makes you look like a jerk and teaches the developer nothing

What you say in code review matters, and so does how you say it. It’s important to be clear and conscientious when giving feedback, positive or negative.

  • One of the first tips I’ve learned when it comes to language is to use “we”, not “you”. “You need to do it this way” comes across like a demand, “we need to do it this way” is a reinforcement of team standards. It’s a small thing really, but can yield positive results.
  • Ask what, not why. People are more open to negative feedback in the form of a “what” than that of a “why”. Rephrasing questions to ask “what are the benefits of x over y” instead of “why did you choose x?” forces you as the asker to clarify what you really want to know, and gives the responder a straightforward, non-judgmental question to answer.
  • Avoid using “just” or “simply”. What seems simple to you might be less intuitive to others — there’s no reason to imply that there’s something wrong if a solution isn’t obvious to them. The reality is that “just” and “simply” are filler words that don’t add value to your comment, so you can leave them out without losing anything.
  • Lastly, emoji. This may be a hot take but I don’t think emoji belong in code reviews. If you’re adding a smiley face to a comment to try and defuse tension it might cause, you should probably reword your comment. Or better yet, deliver it in person! As I mentioned in part 2, people are more likely to take a written comment negatively than a verbal comment, regardless of its content.

It will take some extra effort, but if you are mindful of the language you use in code comments you will see improved results.

Time matters

3 days is a long time to go with no comments or approvals. Don’t make people chase you down to do reviews!

Your time is important, and so is the time of the engineer whose code you’re reviewing.

  • You should do your reviews in a timely manner or let them know if there’s a reason you can’t. If you’re done leaving comments and the code needs work, reach out to let them know it’s time to make edits or mark the PR as Needs Work. Don’t let your code reviews sit in limbo for extended periods of time.
  • Also along the lines of being respectful of time: recognize when an issue you’re about to raise isn’t related to the ticket being worked on, and file a separate ticket to address it. Try not to fall into the trap of asking “While you’re in here, could you just…?” That’s a slippery slope to go down and can turn a simple PR into a behemoth. Not to mention it can make debugging more complicated in the future. It’s frustrating to be looking for the root cause of a problem and have to dig through a bunch of extra changes in a commit to get to what was actually supposed to be in there.
  • And as with being reviewed, as a reviewer it’s important to know when to take a conversation offline. If you’ve gone back and forth twice and a question hasn’t been resolved to your liking, it’s time for some face time.

By respecting others’ time you’ll provide an example of how they can respect yours and foster a sense of goodwill within the team.

Learning matters

Now we all know more about CopyOnWriteArrayList, and knowing is half the battle!

Code reviews are learning opportunities for everyone involved, regardless of their relative seniority on a given team.

  • Be on the lookout for impressive bits of code, and call them out with a comment when you see them. This puts you in a learning mindset to hopefully find something new and interesting to comment on, and provides positive reinforcement to the author of the code. A simple “Clever use of data structures here!” lets the reviewee know they made the right decision. The act of leaving a comment also makes it more likely that you’ll remember their use of data structures when you have an opportunity to do the same in your own code.
  • Try to phrase comments in the form of specific questions whenever possible. It allows the developer being reviewed to maintain a sense of ownership over their own code, rather than just following directions given by reviewers. “This needs work” is NOT HELPFUL. “Does this need work?” is a question, but still not helpful. Be specific and use questions to guide the developer to the answer à la the Socratic method.
  • Questions don’t have to always be about problems you see in the code; you can ask questions for your own understanding. The developer who wrote the code likely put a lot of time into learning the lessons that helped them get it finished. Allow them to share that knowledge with you, but keep in mind the guidelines about when to turn a conversation in code comments into a face-to-face discussion.

As a developer you have an obligation to make sure your code is robust and readable. As a reviewee, the responsibility lies with you to clarify and communicate, as well as address issues that have been pointed out in a timely manner. As reviewer, your job is about more than just finding mistakes. Help your coworker to become a better programmer by conscientiously pointing out possible issues and also pointing out pieces of code that are really well written. While I admit I don’t follow my own advice 100% of the time, things tend to go more smoothly when I do. I sincerely hope you’ve found this series useful! Please feel free to share comments about code review tips that work well for you.