Better Code Reviews Through Empathy
Code reviews are an important part of the modern software lifecycle. Unfortunately, a lot of cycles are burned and morale is damaged because there are few guidelines given to reviewers (and reviewees) on constructive feedback and effective written communication. Below are some tips for a better code review process.
Determine why you do code reviews
Is it to find bugs? Is it to spread knowledge of the codebase? Is it to find architectural problems? What do code reviews offer your team? If you don’t know the answer to this question, you don’t know how well they’re working.
It’s probably best to have this explicit conversation with your team. It’s OK that teams use them for different things, but it is very important that both the author and reviewer know what the expected outcome of a review is.
Within our team, I expect that I’m sharing code mostly to spread knowledge of changes to codebases, and as a sanity check that I’m doing nothing incredibly stupid. Sometimes I will need expert guidance (for example, in front-end work, or in services that I rarely poke around in). Usually, I try to get this before tagging someone in a PR. When this happens, I think it is best to acknowledge explicitly that you’re expecting that person to be the expert, so that they can take a closer look and perhaps help you with a deploy.
Automate your style guide
If the answer to, “why you do code reviews” includes, “making sure the code follows a consistent style,” cut it out. It’s difficult to review code that is poorly formatted. And it’s really, really tough to review code for both correctness and style without making multiple passes. It’s also really demoralizing to new engineers on a project to look at their PR and see dozens of style comments. Great automated style tools exist that can automatically style your code and you should use these. On my particular team, we strive to use Scalariform for our Scala stuff and Rubocop for our Ruby projects. Under no circumstances in 2017, should engineers be manually fixing formatting issues.
As a fallback, in cases where automation has not been set up, it’s best for the reviewer to correct the formatting themselves instead of adding comments to the author (this is doubly true when you are reviewing code for your own teammate). Small things can be handled directly in the GitHub UI and we shouldn’t be shy about adding a few pointless commits that’ll be squashed down before merging anyway.
Offer suggestions, not orders
Don’t tell a committer how they’ve done a thing incorrectly in a code review remark. Offer a suggestion, and ask what they think of it. For example, take a look at the comments below:
Don’t repeat this here, extract a function.
This clearly feels very negative. The author may feel defensive and start a pointless argument, even if there are few benefits to doing so; or worse, the author may feel like a less valuable contributor, and follow the suggestion blindly. In either case, actual harm is done by comments like this.
The best thing you can do is to offer a suggestion, not as a gatekeeper, but as someone who is trying to learn about the changes to the code (even if you really are a gatekeeper).
What do you think about taking this with the above and extracting a function?
Why should you make your comments sound so tentative? Because you are reviewing your peer’s code. You have to invite them to politely explain what’s going on, or to notice that you’ve made a better suggestion. Having no manners in this regard eliminates the possibility of discussion for a significant segment of the population.
Let’s talk about my number one pet peeve in code reviews…
Why didn’t you…
Asking a “why didn’t you” question implies a great many things, none of which are constructive or positive. It assumes that the author of the code has previously considered your suggestion and explicitly chose a different one, which may not be the case. It implies that your alternative is obviously superior (especially in the special case of “why didn’t you just…“), and that you require that the author justify their solution against it. It implies that what may be intended as a mere suggestion is, in fact, the default.
Whenever I am reviewing code and my hubris needs to be checked, I try to offer a suggestion. “How do you feel about…” or “what do you think about…” are good drop-in replacements for this phrase.
Use overtly positive language
It is important to recognize that written communications have a profoundly negative bias. We tend to interpret neutral communications as negative, and positive communications as neutral, which is a source of a lot of consternation in code reviews. To combat this, strive for overt positivity.
Terseness can be interpreted as rude or inconsiderate. In particular, avoid adding a two or three word comment.
Use code reviews as a learning exercise
My genuine unsupported-by-data opinion is that code quality and correctness are only marginally improved by having a code review by a gatekeeper. It is rare that a reviewer shares the same level of insight as the person who authored the code (well, except in the aforementioned, “review by expert”). A better model is to have code reviews treated as a learning exercise for the reviewer, i.e. to understand the changes to the code and to grow in understanding of the code base. I think this leveling gives their questions and remarks appropriate weight, and allows for a healthier discussion about the larger codebase and how the changes interact with it.
Of course, you don’t have to do any of that to be nicer in your reviews.
When receiving a code review
For receiving a code review, you should follow the same suggestions in reverse. Recognize that terse comments don’t mean the reviewer thinks less of you. Recognize that “orders” are usually just poorly-phrased suggestions, and an invitation for you to think about the problem a (potentially different) way. Recognize that pretty much everything written comes off more negatively than intended, and give a lot of leeway to your teammates.
It’s easy to imagine that brutal code reviews are a ritual that leads to the best outcomes. In my experience, engineers are a lot more sensitive to criticism than they let on, and they are a lot more willing to be found wrong if they feel like collaborators rather than defendants. Take a few extra minutes to add some kindness to your reviews, and I think you’ll find your reviews are a lot more productive and leave you feeling better at the end of the day.
*This post was originally featured on Cory Chamblin’s personal blog. We thought it would be helpful to our blog readers, so we decided to share it here as well.