Code reviews are a crucial component of the software development process. It encourages the software development team to interact with one another, collaborate, and improve the quality of their code. However, it’s more than just checking new code to detect errors and become familiar with the codebase.
According to Phil Hughes, front-end engineer at GitLab, it’s about how you provide and convey that feedback — and that’s an art form and a skill that is learned over time. “Reviewing code efficiently is a skill that gets learned the more you do it. Spending time coming up with a workflow that works for yourself is just as important,” he wrote in a blog post.
Here are some tips and tricks to perfect your code review process and to help teams get on the right path:
Decide how you want to do code reviews: According to Sandya Sankarram, software engineer at SurveyMonkey, there are three ways to do a code review: 1. Group walkthroughs where code is reviewed as a group in person. The benefits of this is the ability to debate about concerns and feedback in real time. 2. Pair programming where two developers work together with one writing the code and the other observing and offering advice along the way. 3. Pull request code reviews where code reviews happen offline; reviews, changes and feedback can be added line by line, and feedback can be given through commits.
“It’s like having a proofreader look at what you’re writing and hopefully they see something you don’t see. Sometimes it is about reviewers having more expertise of the particular file and a particular repository so they can give you extra context on the thing you might be changing or identify a potential issue,” said Jarred Colli, head of product for Bitbucket at Atlassian.
Manage time effectively: One of the top challenges of code reviews is making sure developers allocate enough time to review each other’s code. “It’s not uncommon for somebody to submit a pull request and wait two weeks for their teammates to give them feedback before it can be merged,” said Colli. “Ultimately, the goal for development teams is to merge high-quality code quickly. Some people err on the side of making sure the code is high quality. Some people prefer to err on the side of making sure that it’s done quickly. The goal is to try to do both.”
One way to do both is to make sure code doesn’t stay unmerged for long periods of time. The less time it stays unmerged, the less opportunity it has to change or cause potential problems, according to Colli. Colli added pull requests should be reviewed by teammates within a couple of days and then get merged.
According to GitLab’s Hughes, individual developers should find a time in their daily routine that works best for them. For instance, he finds starting his day off with code reviews for him provides less distractions and more productivity. Additionally, Hughes says to have an agreement for how long code reviews should take. GitLab has a 2-day Service Level Objective for reviewers to get feedback to developers.
Keep it short and simple: There is an inclination to put all changes together in one large package and then make it available for everyone to review. This makes the process confusing because there could be multiple files changing simultaneously, making it hard to effectively review the code. It also reduces the likelihood team members are going to find problems. “Developers are so busy with a million different things, it’s hard to sit down and do that all at once. They end up having to come back to the review multiple times, they might skip a file, lose their place, or forgot what they read before,” said Colli.
Tightening the feedback loop between writing the code and getting feedback also prevents developers from getting too attached to the code and then getting defensive over changes being made to it, according to Erik Dietrich, a programmer and co-founder of the technical content marketing firm Hit Subscribe. Pair programming works well here because developers are closely aligned so when they make a mistake they can immediately get that feedback.
Automate when possible: According to Aki Asahara, CEO of the software development company Sleeek, about 30% or more of a team’s working time is made up of code reviews. By being able to automate parts of the code review, it can save time and enable higher-quality code. Asahara explained that a majority of code reviews involve just checking the state and style of the code. With an automated tool, teams can write custom policies or rules so developers don’t have to worry about spellcheck or repetitive tasks, and they can deal with more essential parts of the code.
Atlassian’s Colli explained there are a lot more automated tools coming out to help support and reduce the amount of time it takes to perform a code review. Code insight tools can help identify dependencies and issues to let developers know if things need to be updated and provide more context around code that is being changed. Security scanning tools can help find potential vulnerabilities.
Having automated tools also takes pressure and conflict off developers and teammates, according to Dietrich. He explained developers tend to have negative connotations towards reviewers because they feel like they take too much time or nitpick their code. “And beyond the interpersonal dynamic, automated reviews are relatively infallible. [Automated tools] won’t miss something because they haven’t had their coffee yet, or because they’ve been at this for hours, it’s Friday, and they want to knock off for the day,” said Dietrich.
Have a checklist: Atlassian’s Colli explained it can be difficult for teammates to confirm that the reviews have been done and the suggestions and feedback were appropriately acted upon. A checklist could include whether or not the code reviewed feedback; was anything done about it; was it done the way it was suggested to be done, why or why not?
Take advantage of merge checks or branch restrictions: Similarly to custom policies, merge checks or branch restrictions can set conditional rules in place before a pull request can be merged. “The more structure that you can add to the back and forth between teammates providing feedback, the better off you are going to be,” said Colli. For example, a check can be to make sure the code was checked and approved by at least two reviewers before it can move on; or if a security vulnerability is identified by the security scanning tool the code won’t be merged.
Take advantage of talent: One strategy is to find developers who understand the complexity of the code or who are an expert in the area of the code being checked so they can provide feedback about any risky changes, according to Colli.
Use code reviews as a teaching opportunity: Code reviewers help developers become more familiar with the codebase, but it’s also an opportunity for senior developers to help junior developers by telling them how they could have been more effective. “Few people realize the impact on knowledge sharing and mentorship code reviews have on a team. They are a tool to help engineers grow faster (junior engineers see seniors’ code more frequently), get better at mentoring (giving constructive feedback),” Gergely Orosz, an engineering lead at Uber, wrote in an email to SD Times.
Without code reviews, the software development process can suffer from “less knowledge sharing, more easily preventable mistakes going into production,” and code that is “often less readable as a result,” he explained.
From meaningless to meaningful code reviews
Code reviews can be a source of anxiety and misery for groups that are more sensitive and appreciate polite gentle feedback. But on the other hand, code reviews can also be great for groups of people who appreciate direct and honest feedback, no matter how blunt or brutal it may be. “For the latter group, code review is great; it’s a chance to duke it out over strongly held personal beliefs with (theoretically, in their minds) no hard feelings,” said Dietrich.
It doesn’t have to be one way or another. According to Sankarram, there is some middle ground.
In a talk on “Unlearning toxic behaviors in a code review culture,” Sankarram pointed out some toxic behaviors she and others have experienced as well as provided recommendations to improve code review culture.
“Please keep in mind that I’m not discouraging feedback. I’m just making a plea for people to govern their tone and the way that they choose to deliver the content of their feedback,” she said in her talk.
Don’t pass your opinion off as fact: Unless you can provide proper resources and documentation to back up your claim. According to Sankarram, this is common when it is a question of style and syntax. She recommended implementing a linter or automatic code fix so team members can see what style guidelines they should be following. “You should be fostering debates and not making demands,” said Sankarram.
Dietrich explained saying “that’s wrong” or “that’s bad” can cause developers to be defensive. “Contrast that with a statement like ‘that’s not what I’d have done there,’” which is essentially inarguable. Even better, though, I’ve found, is to ask questions instead, like ‘why did you choose that implementation,’ or ‘couldn’t that lead to a race condition?’” he added.
Using opinions instead of fact can also waste developers’ time arguing over subjective concerns. “If you have domineering reviewers that are providing feedback that’s completely non-actionable or arbitrary, it creates learned helplessness in the subjects of that review. They learn to do a pretty minimal amount of effort, since the domineering personality will pick apart everything they do anyway,” said Dietrich.
Don’t leave an overwhelming amount of comments: “When a person makes an error, chances are that they’ve made the same error in several places,” Sankarram explained. Instead of leaving comments in every single place the error occurred, simply leave detailed notes with helpful links and resources so you are conveying your message, but not overwhelming the developer.
Don’t ask reviewers to fix problems: If there are issues within the code that aren’t directly related to the reviewer’s change, it becomes a separate discussion.
Don’t ask judgmental questions: This is unhelpful and implies that the developer should have known about a simple solution. It also forces the developer to have to defend their work. Instead, you could recommend they do it a different way rather than using harsh and judgmental words such as “Why didn’t you do it this way?” said Sankarram.
Don’t be sarcastic: Code reviews are not the time to be sarcastic, it is a time to offer someone feedback. “Saying something like ‘Did you even test this code before checking it in?’ is sarcastic, it’s rude and it gives no context or actionable feedback. It is better to describe the problem and not waste time,” Sankarram explained.
Don’t use emojis to convey feelings or point out issues: For instance, Sankarram has seen the thumbs-down or puke emoji used in previous code reviews. Emojis, while fun, are cryptic and easy to misunderstand as well as wastes people’s time trying to figure out what the emoji means.
Don’t ghost people: Developers seeking code reviews can also contribute to the toxic culture by not responding to feedback, leaving people wondering why they even bothered to help you in the first place. If you don’t agree with something or are not going to make a particular change, let the reviewer know why.
Don’t ignore toxic behaviors: It’s typical to deemphasize behavior especially if it is coming from a high performing or productive developer, according to Sankarram, but these developers still need to understand that their behaviors are stressful and draining and not helpful to other team members. “The stakes are pretty high, not taking steps to unlearn these toxic behaviors have serious setbacks for the team as developers feel overwhelmed, attacked and unmotivated. They start to draft these feedback processes that are supposed to help them grow,” Sankarram said.
Dietrich also suggested code reviews should be bidirectional and includes all team members regardless of the experience level. Also, letting team members pick their own code reviewers can help them be more comfortable with the review.
“You want to make sure people feel safe to take risks and get feedback, and not be scared to get negative feedback or try something different. Every team needs to start with figuring out how to build that trusting environment among other collaborators so that kind of bleeds through all the work that you do together,” Atlassian’s Colli added.
