Honing the effective Pull Request (PR) review is an important skill that makes you a valuable teammate. It can be overwhelming, especially in scenarios of major refactors or highly unfamiliar business rules. Scrolling through a diff view is mind-numbing and not helpful if you haven’t internalized all the surrounding unchanged code’s context too. So how do you go from spending large blocks of time combing through every detail of a PR to maximizing a time-efficient, deep comprehension of the code you are reviewing? I asked some of my coworkers about their strategies, and here’s what they had to say.
Starting from the tests is a bad approach.
This was the most surprising input I received from my coworkers. I always thought that looking at the tests first would provide me with guardrails pointing toward what to look for in the implementation code. As it turns out, this approach is backward.
Reviewing test names can offer explanations of expected behavior, giving context to the functionality being implemented. However, it’s essential to remember that tests can sometimes be poorly constructed. They might pass without thoroughly evaluating the underlying implementation, leading to a false sense of security.
This is less strictly true for things like integration tests or snapshot tests, which should reflect the holistic functionality of your application. Regardless, avoid relying on tests for an understanding of individual units of code.
Avoid losing the forest for the trees.
Reviewing a PR only within the context of a line-by-line diff view can be time-consuming. You might also fail to consider related code that was not altered in this set of changes.
Whenever possible, pull down the code and walk through the entire code path. This thorough exploration ensures you’re not overlooking critical aspects that might not be immediately visible in the diff view. By doing so, you can provide more meaningful and comprehensive feedback.
On a team with an attitude of continuous improvement, code outside of the immediate changes should still be included in feedback and consideration.
Ask for a code tour.
At Atomic Object, we value frequent pair programming within our teams. This can extend to PR reviews too! Pair code reviews, or code tours, are invaluable, especially when you’re unfamiliar with the codebase or in the scenario of a large refactor or other major change. Hearing directly from the code’s author allows for real-time clarification and a deeper understanding of the rationale behind certain implementations.
Code tours can also spotlight areas that are ripe for improvement. If you hear the author expressing things like “I almost went this route… I don’t really love this solution…”, it is the perfect opportunity to provide feedback that will certainly be valuable.
Pair PR reviews are also a chance to build trust among the team. They create opportunities for knowledge sharing, mentorship, and fostering a supportive environment where team members feel comfortable discussing and refining their work. Any text tone barriers that come up in asynchronous reviews are removed.
There’s no need for extensive comments.
I feel a lot of pressure to leave extensive, well-thought-out comments for even minor suggestions. However, this approach can overwhelm the developer and dilute the impact of your feedback. Instead, focus on providing clear and concise comments that address the most critical aspects of the PR.
Sometimes, a simple “What’s this?” comment is sufficient to indicate that a particular section of code needs further clarification. If something does lead to a paragraph-long comment, that’s a good indicator that you should pivot to having a real-time discussion.
Effective pull request reviews are integral to maintaining high-quality code and fostering a collaborative team environment. By shifting your focus from solely tests to a more comprehensive review approach, avoiding the pitfalls of line-by-line scrutiny, engaging in collaborative reviews, and prioritizing clear communication over exhaustive comments, you can enhance both the efficiency and effectiveness of your PR reviews.
Remember, the goal of a PR review is not just to identify issues but also to facilitate knowledge sharing and continuous improvement within the team. Implementing these practices can lead to more meaningful feedback, stronger team dynamics, and ultimately, better software.
Have you adopted any of these strategies in your PR reviews? Or perhaps you have other techniques that have worked well for you? I’d love to hear your thoughts and experiences in the comments below!
I just started reviewing Pull Requests a few months ago, and I find that sometimes I get caught up in the details and lose sight of the big picture. I’ve been experimenting with checking the overall logic and flow before diving into the code. But this post makes me think that asking for a ‘code tour’ or working with a team in person can be much more effective. I’ll try that in my next review!
This is a great article! I learned a lot about effective pull request reviews.