Code Review Best Practices That Will Boost Team Morale
The code review is an important form of collaboration for developers, used for everything from assuring the quality of open-source contributions to sharing knowledge across teams within a single software company. But the process can be intimidating for those who have never done one before.
For one, code reviews decide whether a developer’s pull request gets included in the codebase. And for some devs, it can be scary to have other engineers comb through their code, actively looking for mistakes.
Lisa Scheibner, who majored in English in college and now works as an engineering team lead at Allstacks, compared the experience to attending creative writing workshops.
“It’s a terrifying experience to take your own work and put it out there, and have people tell you, ‘Here’s what you need to change,’” Scheibner said.
Ways to Make Code Reviews More Constructive
- Comments should be about the code, not the developer. Programming skill evaluations don’t belong in code reviews.
- Don’t ask condescending questions. Common examples of the genre include “Why did you do this?” and “How long did this take?”
- Write clear and specific comments. Try referencing specific function names when possible.
- Include positive feedback. Surely your fellow devs are getting some things right.
- Avoid nitpicking. If consistent style is important to your team, use automatic linters instead.
- Build rapport outside of code reviews. Code reviews work best when built on a foundation of trust.
Most developers quickly get used to it and learn how to incorporate suggestions from others. After all, code reviews are meant to be opportunities for mentoring and growth, for knowledge sharing, project course corrections and catching bugs while the stakes are still low.
But it doesn’t always work out that way.
Code reviews can also be opportunities for bullying, misunderstandings, and a lot of hurt feelings, in which case they can turn into a regular activity that adds to everyone’s stress.
There are ways to prevent that from happening to your team, however. Here’s what you need to know if you want to keep your code reviews friendly and constructive.
Leave Clear and Respectful Comments
Shifalika Kanwar, an engineer at Bloomberg who has given talks about fostering inclusive code reviews, said it’s important to remember that comments should be about the code, not the developer. References to the code author’s programming skills and comments about their familiarity with tech stacks don’t belong in code reviews. It’s embarrassing to be called out in a pull request, which is essentially a public space — instead, reach out through other channels if you would like to provide some extra mentoring.
Kanwar said it can also help to word comments inclusively, such as writing “we” instead of “you.”
“Avoid anything that’s going to either make a code author feel on the defensive or feel like they have to justify the work they’ve put in.”
Asking clarifying questions in code reviews is generally welcome, but software developer Dan Goslen wrote about how certain types of questions can be condescending. These can appear as passive-aggressive rhetorical questions like, “What are you doing?” and “How long did you spend on this?” Goslen said it’s better to point out problems in other ways, such as by giving specific suggestions on how to change something.
“Avoid anything that’s going to either make a code author feel on the defensive or feel like they have to justify the work they’ve put in,” Goslen said to Built In.
Comments should also be clear and easy to understand, to avoid confusion and misunderstandings. Scheibner reads over her own comments to make sure they make sense, and also tries to make them as specific as possible. Sometimes that involves pointing out exactly what part of code she’s referring to.
“Sometimes it’s not clear enough to me when you put in the comment what code it might be referring to, so sometimes I’ll use function names to be sure that it’s clear,” she said.
Comments in code reviews don’t all need to be critical either. It can be nice to receive words of encouragement and praise in there too, Kanwar said.
“I find that everybody likes compliments and positive feedback,” she said.
Code Reviews Should Balance Thoroughness With Efficiency
A lot of the tension around code reviews can be traced back to disagreements over the types of changes reviewers should be requesting. Certain comments are always welcome — ones that point out outright errors and bugs, such as code that calls the wrong library, for instance. But when it comes to questions of style, syntax or readability, finding the right balance can trickier.
Some developers prefer reviews where comments on any aspect of the code are fair game, to ensure the best code possible. There’s a logic to that method, but it doesn’t come without some costs. Code reviews are time consuming, both for reviewers and code authors, and every additional round of comments and responses takes energy and time away from writing other code. And code that’s being reviewed may also be blocking other pull requests that depend on it, creating review bottlenecks.
“Getting the code through review as early as possible, because there might be changes that you have to make, is super important,” Scheibner said. “Having a review blocked can have serious negative impacts in terms of how fast things move.”
Code Reviews Can Affect Relationships
Although work efficiency is important, Daniel Lew, a software developer based in Minneapolis, said the impact on team relationships was an even more important reason to limit the scope of comments in code reviews.
Even after many years of software development and code review experience, Lew has found that getting critical feedback can still be difficult — even when the feedback is obviously helpful and well intentioned.
One instance happened recently, when a reviewer asked whether Lew had considered another way of writing an algorithm that was superior. Lew had spent significant time on writing the algorithm, and his initial feeling upon receiving the feedback was defensive — he had to actively stop and force himself to think about the reviewer’s comments before accepting that it was right and he should make a change.
“People feel emotions, and people feel emotions over the things they create.”
“It’s hard to do that,” Lew said. “People feel emotions, and people feel emotions over the things they create. If everyone could just view this as automatons, then that would be the best, because then you would just have robotic discussions over what would be the best code — but that’s just not how most people work.”
Having pride in and feeling some defensiveness over one’s work is a normal, human reaction. That’s not to say developers shouldn’t be receptive to others’ feedback; people will overcome their pride and do what’s best for the team — but only if they really feel that it’s for a good reason. If they feel pressured into making meaningless changes, relationships can quickly sour.
“The whole goal is that when I ask you to change something, it’s because it’s important,” Lew said. “When people feel that, it’s a much more friendly interaction.”
A couple years ago, Lew’s team decided to try an experiment. Up until then, Lew was a self-described nitpicker, commenting on everything in each pull request that wasn’t up to his standards of ideal code. The team decided to try and relax its approach to code reviews for a while, forbidding nitpicking in code reviews for a month. Lew was skeptical — he suspected that code quality would be negatively affected and the number of bugs would skyrocket — but he went along.
To Lew’s surprise, the experiment went so well the team decided to extend the practice indefinitely. The code didn’t suffer and the team even began to work better together. In hindsight, Lew said it made sense that code quality wasn’t affected by comments asking for changes to optimize variable names and adhere to stylistic preferences.
“The bugs that are written aren’t caused by small stylistic problems, and it’s not fixed by doing these small refactorings,” he said.
These days, Lew focuses his energy on reviewing bigger-picture things, such as the project architecture and actual errors in the code. He doesn’t spend any time optimizing other developers’ variable names, and will only reach out through other channels if he notices a pattern he thinks should be addressed.
“The bugs that are written aren’t caused by small stylistic problems, and it’s not fixed by doing these small refactorings.”
“I’m much less worried about individual lines of code causing problems than about, like, us steering the ship in the wrong direction and then having a hard time correcting it,” Lew said.
For teams that want to stop nitpicking during code reviews but still want to set standards of code style and readability, there are better alternatives. Teams can set aside time to talk about those standards, then set up an automatic linter to highlight and fix code that veers off from the agreed-upon style guide.
Establish Relationships Outside of Code Reviews
But try as you might, sometimes feelings will still be hurt during code reviews. Language can sound different online, and some people just have a more abrupt communication style.
“Because the internet, right?” Goslen said. “I’ve totally seen that happen. And I often see that happen in code reviews that are across teams, where the author and reviewer don’t work closely on the same team already.”
Goslen said the solution is preventative — ideally, the team should carve out opportunities to get acquainted outside the context of code reviews so they can gain an intuitive grasp of the other developers’ intentions.
“It’s important to create that foundation of safety and trust before these things come up in code reviews,” he said. “You learn how each other work and how each other talk, and that’s important if you can build rapport outside of the review process.”
This can involve socializing, interactions such as daily meetings or working together on code in other ways. For instance, team members can pair program together.
“It’s important to create that foundation of safety and trust before these things come up in code reviews.”
Pairing can also be an alternative to writing everything in comments during code reviews. If a comments thread starts getting too long, bringing the conversation offline to a different channel can save time and prevent animosity.
“Maybe a pull request is just taking a really long time and there’s a thread of five or six comments,” Goslen said. “And you’ve probably created frustration and tension without meaning to. Moving that to a Slack conversation, or even a video call, is going to save you time and probably bring down tensions and frustrations.”
Goslen said the real goal of code reviews is building knowledge and relationships, not just fewer bugs. That’s why it’s so important to do them well and create a beneficial learning environment with the review process for everyone.
“The point of code reviews is about understanding ... a way to learn more about my team, the software we’re building, the languages we use, what our customer needs are,” Goslen said. “The result of that is less bugs, higher quality code, more stability — but they’re a result of a really good process about trying to really understand what the code’s doing.”