Giving Better Code Review Feedback
Have you ever been met with an emotional reaction when you asked a logical question? This happens all the time in code reviews. They easily get personal, even when it’s about the code.
As a reviewer, it’s hard to see the history of constraints and tradeoffs the author is working with. What’s in the PR might be the 5th approach to solve the problem, because the first four tries led to dead ends. To the author, the points you raise for discussion can easily seem like you’re not understanding the constraints the person is dealing with, or simply that you think you know better. It’s a recipe for defensiveness and misunderstanding.
Fortunately, there are a couple ways that you can frame your PR comments so that your intention is clear, leading to more productive code reviews.
Check your mindset
It’s easy for your feedback to flow quickly from your head to your keyboard. Take a moment to think about your own mindset. Are you pissed off? Annoyed? Confused? That’s going to show up in your review. These reviews are open for the rest of your team to read, so think about how you want to show up.
Know if you’re seeking to understand, or seeking to correct
Get clear on the purpose of your comment before you press Send. Are you genuinely curious, just need more information, or need something to be fixed before it can be shipped? Not all review comments need a long back and forth. If you’re seeking to correct, be polite and clear. “Check our style guide here for details on how to organize routes.” If you need more information, make sure your questions are formulated in a way that gives the author space to explain, and doesn’t put them on the defensive from the start.
Some question formats are judgemental
“Why didn’t you use threads here?”
A question phrased like this puts the author on the defensive straight away. While your intent may be to understand why they chose one approach over the other, what you’re asking them to do is to defend their choice. Embedded in this question is a judgement — that you think threading would be a better solution, or even that you suspect the might not have considered the solution at all.
Try instead: “What other approaches did you consider?” or “Do you have some data about performance when doing this single vs. multi-threaded?” These assume that the author already considered threading, but chose a different approach for a good reason.
“But what about X?”
This easily translates to “I think you forgot about X.” Maybe that’s what you mean, and maybe that’s what happened. But if the author carefully considered X, but had to choose a different approach, your question can easily seem accusatory.
Try instead: “I’m curious what happens when X.” or “Coverage of X edge case isn’t obvious to me from the code. Can you explain?” If they forgot about it, they’ll likely come back with "good catch, thank you.” Same result in the end, but a more positive interaction.
Move to a different discussion format
For most of us, it takes longer to type than to speak. Sometimes the format of the discussion can get into our way. Adding to that is the fact that words are just one part of communicating. Body language, facial expressions, tone of voice — these all add meaning to what you’re saying. If you’re hitting a dead end when discussing in the PR review, or if the feedback is relatively complex, switch to a different format. Just make sure to take notes and share them in the PR so you have a record of the discussion. Your future self and your future team will thank you in 6 months when no one remembers the conversation anymore.
