Before any code goes into production at internet testing company Ookla — whether it’s a simple feature or a complete code overhaul — it must go through a pull request. Likewise at cloud platform company DigitOcean, where any change to the code base must be reviewed by at least one other engineer on the team.
Most engineering teams have some form of code review in place. In fact, the process has only grown in popularity each year, with SmartBear’s most recent “State of Code Review” report finding that 67 percent of its 1,100 respondents are doing some form of review, up 10 percent from 2017. Still, just because a team commits to having a review process, that doesn’t mean it’ll be effective.
When poorly implemented, reviews can be an excruciating process where an engineer feels judged or nitpicked by their colleagues. Even worse, teams can treat it like a formality, rubber-stamping code without taking the proper time to check for quality.
“In every case that I’ve seen it go wrong, it’s either someone being very rigid about trivial matters ... or the comment is ambiguous.”
Navigating a productive code review can be a tricky experience, even for seasoned engineers like DigitalOcean’s Billie Cleek. Make a terse comment or misinterpret a recommendation, and the interaction can quickly devolve into a trivial argument.
Cleek knows — he’s participated in a few.
“It usually comes down to people talking past each other,” Cleek said. “Sometimes it can be very difficult to talk about technical details unambiguously. In every case that I’ve seen it go wrong, it’s either someone being very rigid about trivial matters ... or the comment is ambiguous.”
There’s no fool-proof method to ensure a perfect code review process. Instead, the best reviews require mixing technical expertise with thoughtful communication, Cleek said. When done well, the process can be a learning tool for engineers at all stages, foster a stronger team culture and improve the quality of the code base.
“It’s an important efficiency for companies, and for individuals. It’s useful because it can be a process by which you strengthen your relationship with coworkers and broaden your reach in the company,” Cleek said.
Whether you are the reviewer or the submitter, here are a few tips from Cleek and Ookla’s web development lead engineer Chris Sund that could help you be more effective in your review process.
Three Tips For More Effective Code Reviews
- Pairing a newer engineer with an experienced one on a code review can make the process more effective. Newer engineers can bring fresh perspective, while experienced ones can catch redundancies and style errors.
- Err on the side of overcommunicating during a code review, until you have a relationship with the engineer. Try parentheses to note personal preferences, ask questions to clarify and don’t be afraid to outline your thought process to avoid misunderstandings.
- Clear naming of functions and a descriptive commit message are ways the submitter can facilitate a more thoughtful review.When the reviewer knows what the code is trying to do, they can provide more effective feedback.
Put teams in a position to do effective reviews
Every engineering team has a different code-review process.
At Ookla, Sund has found that pairing a newer engineer with a more experienced one creates a teaching opportunity and room for fresh perspectives. Sometimes, the newer engineer may spot a mistake that the senior engineer glossed over. Meanwhile, the senior engineer brings a deeper knowledge of the code base, which allows them to catch redundancies and complexities in the code.
“It’s important that we’re holding the quality of our code high and delivering features in a timely way.”
Small steps like those have helped his team to ensure the review culture is passed along to newer team members and that the engineers are able to provide quality feedback.
“It’s important that we’re holding the quality of our code high and delivering features in a timely way,” Sund said. “But if we can, we’ll take opportunities to learn and teach.”
Sund said a good review takes less than an hour to complete, although that fluctuates with the amount of code being reviewed. When it comes to larger implementations or code overhauls, the code is often broken up into smaller chunks so it can be reviewed more quickly.
Distinguish between necessary edits and personal stylistic preferences
When it comes to reviewing code, it can be tough to know what’s a necessary change and what’s a personal preference.
That gray area is what Sund calls the art of the review process. On his web development team, all code must pass its security protocol. If it fails its security linting program or is poorly formatted, causing a vulnerability in the code base, it’s an automatic change. After that, however, it falls into a gray area, where edits are open for discussion or could be tweaked to better fit the existing code base.
“Finding ways to not repeat code across the platform is huge for every development team out there.”
Sund looks at the quality of the implementation and opportunities to limit complexity or simplification. He’ll also mark opportunities to reuse code someone else has already written when possible, or make code more reusable, which helps to reduce redundancies in the code base.
“You have to decide, well, this works for this specifically, but is there a way we can design this so that it can be reused in other places,” Sund said. “Finding ways to not repeat code across the platform is huge for every development team out there.”
Before making any comments during a code review, Cleek makes sure to read through the code at least one time to understand what the engineer is trying to accomplish.
After the first read through, he looks to make sure the code is functionally correct and that it doesn’t interfere with another feature. He’ll also look for readability, making sure the code is clear and easy for another engineer to refer back to or build on.
Not everything is a necessary change, however. Often, reviews can be derailed over superficial details and stylistic preferences. To keep that from happening, Cleek encourages his team to use parentheses to denote personal preferences and stylistic suggestions not critical to functionality. He’ll also make it clear when something is only a suggestion and not a necessary change.
Code reviews are dialogues, not one-sided conversations
It’s not uncommon for engineers to tense up after hearing the words “code review,” for understandable reasons, Cleek said.
While code reviews are designed to improve the quality of code, it can often feel like a personal critique. That’s why Cleek believes the most important aspect of any code review is communication.
“As engineers, we tend to focus on the technical details and jump straight to the point,” Cleek said. “But when you’re developing a relationship with someone — especially someone new to software — they can be kind of possessive of their code. We want to foster a sense of ownership without possessiveness.”
At the leadership level, Cleek said it can be helpful to define the purpose behind the code reviews to clarify that it isn’t a judgment of the engineer but of the code itself.
Within the review process, Cleek reads code all the way through before commenting, to keep from jumping to conclusions. From there, he tries to tailor his message to the submitter. It can be easy to jump straight into technical changes, but Cleek knows that can make some engineers, especially newer ones, defensive.
“I always look to learn how I can improve my written communication in a way that strengthens a relationship instead of weakening it.”
To combat that, he’ll share his thought process behind his edits, dot his comments with emojis and offer positive feedback. Once he develops a relationship with an engineer, he’ll be more direct in his comments.
“Being sensitive to the communication styles in your relationship with people is important when you’re doing code reviews,” Cleek said. “I always look to learn how I can improve my written communication in a way that strengthens a relationship instead of weakening it.”
For any review, Cleek’s goal is to make it a dialogue. He asks questions about the code if he doesn’t understand something, and he makes it clear he’s interested in the answer and not challenging the submitter. He also points out when he’s learned something new and shares what he liked about the code. If something isn’t clear, he hops on a video call.
Sund said developing a relationship with the other engineer is critical. His team is remote, so taking a few minutes after a review to have a conversation and understand the other engineer’s communication style is crucial to helping prevent misunderstandings.
It also helps to keep the dialogue professional at all times.
“If you keep the dialogue professional and give people the benefit of the doubt and ask questions when you’re not sure of something, then you can cut down on a lot of that negativity,” Sund said.
Always be clear about what you’re trying to accomplish
One of the most helpful steps an engineer can take to facilitate a code review is to write a commit message, Cleek said.
A useful message should contain a brief summary of what the code is for, why it’s needed and how the engineer approached it. A clear message can make it easier for the reviewer to understand what the engineer is trying to do and make informed suggestions.
“It really eases the burden on the reviewer to understand what’s being done, and evaluate whether or not it’s being done correctly,” Cleek said.
Taking the time to clearly name and define functions and variables is also crucial, Sund said. It’s something Sund has struggled with for years, but he’s found taking a break from the code and then asking himself, “Does this describe itself?” on a re-read helps. If that doesn’t work, then he leaves a note for the reviewer explaining his goal and asking for help.
“The naming of functions and variables is important,” Sund said. “The code should really tell the story.”