As a junior software engineer, I always perused code review comments I received to learn how to become a better coder. But when it came time for me to attempt my first code review, I realized my experience hadn’t prepared me to be on the other side.
I developed a severe case of imposter syndrome, spiraling downward with questions: Should I comment on this line of code or is it not worth it? Should I find articles to support every comment? Will I crash the site by approving this? Will they hate me? Okay, I admit I spiral pretty quickly. But after speaking to some co-workers, I knew I wasn’t alone in my worries.
Junior software engineers may be thrown into code reviewing with an assumption analogous to “you know how to read a book so you know how to write a book, which is not true,” says Jessica Rudder, an experience engineer at GitHub.
There are expectations that come with code reviewing, and the process can be nerve-racking. So I interviewed seven other software engineers to collect tips on how to build a reviewing mindset.
1. Think About the Overall Impact
Generally, a good pull request (PR) should only affect a confined part of the codebase. However, the limited scope shouldn’t prevent you from thinking about the code change within the context of the larger codebase.
Nigel Munoz, a former full-stack engineer at The Muse and a current freelance software engineer, encourages the reviewer to think about “how this change affects the larger and smaller picture.” Considering the larger picture includes finding any technical debt—look for code that is repeated, non-modular, or does not adhere to the most recent standard conventions—as well as analyzing modifications to the architecture of the codebase.
Sam Donow, a core developer at Hudson River Trading, believes that “there is nothing too big or too small to comment on. Suggestions for small improvements could lead to bigger improvements in multiple parts of the codebase.”
For example, during one of my own code reviews, I received a comment that certain properties on a React component were confusing, which sparked broader questions about how the component was being used. Ultimately, I removed the properties from the original component and created a separate component to clarify the behavior of each one and ensure both could be used in more places.
2. Consider Security
Don’t forget that some changes could impact more than just the codebase. Rudder recommends evaluating if a user “could use this functionality to harass someone or could abuse the system.” For example, if the new feature in the pull request includes user entry, look for SQL injection, data access, cross-site scripting, and other security vulnerabilities.
3. Focus on Bugs
Your fellow code contributors—no matter how robotic they may seem—are human, and humans may break or forget functionalities. So make sure you “review the tests with the same importance as the rest of the code,” advises Abhishek Pillai, a tech lead at Teachers Pay Teachers. “They will prevent new bugs and serve as a form of documentation to anyone else who works on this in the future.”
Reading the tests can help you understand the functionality of a new feature and see the different cases it will introduce, while analyzing the tests can help you point out missing cases and find features not contained in this PR. If there are no tests included in the code change and they seem relevant, it is appropriate to request them within the review.
But tests aren’t everything. “Don't put too much faith in the system,” Donow warns. “Just because tests ran does not mean there are no bugs.”
You might also want to “run the app locally to functionally test it and make sure it works. If it doesn’t work, then there’s no point in further reviewing,” says Ryan Verner, a software developer at 8th Light. Though some reviewers don’t think manual testing should be part of the code review process—in part because of the time it takes—Verner believes it’s a quick way to determine if you should invest more time reviewing as well as a strategy to help curtail the growth of a bugs backlog.
4. Be a Team Player
The cliché takes on new meaning when it comes to reviewing code. “Take the time to review because it’s everyone’s codebase,” says Verner, who advocates for a sense of “collective ownership.” You, as the reviewer, should be working toward protecting the backlog of bugs from growing larger with the goal of giving your team less work down the line.
At the same time, Charles Luxton, a tech lead at The Muse, encourages the reviewer to understand and remember the team’s priorities. With fast-approaching deadlines and disagreements abounding, sometimes creating a to-do item for the backlog that ensures improvements will be made in the future and putting a comment on the code in question in the meantime is the Band-Aid you need in order to keep your team happy.
Finally, asking yourself if the code would make sense to someone who had just joined the team and was reading it within their first few weeks will help keep your code readable and understandable.
5. Use the Process for Learning and Knowledge Sharing
The review process gives everyone involved a place to get more insight into the codebase, languages, frameworks, and best practices.
Matt Jeffery, a tech lead at The Muse, advises the reviewer to "understand the changes architecturally. One way is to read file names since they help to give context. For example, if you're looking at a change in the data access layer you know you're not dealing with business logic or UI."
When you learn from code changes, you also have the opportunity to share knowledge. “It’s best to explain your opinion and back it up with documentation,” Luxton says. The links you provide to supporting evidence and trustworthy articles not only help the reviewer and code writer explore different approaches as they make a final decision, but also bolster their knowledge of programming.
While you keep these tips in mind, remember too that reviewing is a time to exercise your people skills. “Give people the benefit of the doubt that they thought about their approach and point out different possibilities while trying to dispel defensiveness,” Rudder says. “I leave comments throughout and a wrap up comment—here’s what’s great, here’s what can be improved, here’s what needs to be changed before merging.”
With this kind of approach, not only will you be protecting your code base from tech debt, security threats, and bugs, but you’ll also be building your team.