The art of pull request reviews

Photo by Yancy Min on Unsplash

The art of pull request reviews

Table of contents

No heading

No headings in the article.

Code reviews are one of the most effective ways of improving the quality of any codebase as opposed to fixing bugs as they come along (rest assured that they will). Some of the issues that people usually look out for in a review include:

  1. Poor design patterns

  2. Code smells, which normally have the potential of introducing bugs

  3. Naming and formatting issues etc

Despite the benefits of code reviews, they are extremely difficult to conduct and time consuming as well from seniors and juniors alike. Think of the number of times the daily has had at least one person saying the plan is to review a pull request. With such importance and difficult, it follows then, that there are strategies we can use to conduct and get good reviews.

Create a checklist

A checklist helps with catching common issues and ensures a standard is maintained across all code reviews by all team members. This structured approach provides quality checks that cannot be underestimated. This list can have things like readability, architecture decisions, design patterns, usefulness of tests, etc. Further, each checklist item can be refined so that everyone in the team, including juniors and new joiners, can easily conduct a review.

Supplement pull requests with automation

What this means is we need to use tools that do some things for us automatically thus sparing some time. Any decent CI system should do the trick. Some mundane and time consuming tasks such as checking for code smells, test coverage, poor formatting of code. The focus then is on the quality of the code, which is the best way to make use of a teammate’s time.

Small pull requests

The team has to make a conscious effort to avoid large pull requests. The definition of large has to be made by the team but ideally keep it under 400 lines changed. Breaking down an epic into small tasks and merging those tasks into a separate branch before merging into the main one goes a long way. For starters, reviews will go faster, the team will feel ownership of the work, and everyone will have a good understanding of what the entire PR entails. If a PR is bigger than the agreed upon size, then it can be automatically rejected by the CI.

Actionable and justifiable feedback

A code review has to be actionable and to the point. During a review, it is simply not enough to suggest that something can be done better. The reviewer has the responsibility to explain how the code can be made better. With markdown support (or similar) for most code hosting platforms, suggestions are easily made.

While this is good, the reviewer has to explain why the suggestion is better than the current implementation using facts because there is usually more than one way of solving problems using code. An added advantage is that all parties save on time because there is no need for a follow up conversation to clarify comments.

Better commit strategy

We have all seen two commits that do different things but have the same commit message. Or maybe there are no duplicate commits but the message has two words like fix tests, which is not descriptive. If you are requesting a code review, you are doing yourself a favour by coming up with a better message strategy that explains what each commit does without being too verbose. Another improvement would be to use the ticket number before every commit message like PCX-2342: update primary colour in payment screen helps to keep the commit history easy to track. Speak of WHAT has been changed in the commit message and nothing else.

Block out time for review

While doing a code review, it is a good idea to spend a maximum of an hour on one pull request. Performance and quality goes down immensely as time progresses. Additionally, it is not a good idea to do a review where the chances of a distraction are high because something is likely to get past this net.

Conversely, a reviewer should not do a code review too fast if the codebase’s quality is to be maintained or raised. Making the assumption that someone else will catch issues or trusting your teammate too much is the first step to shipping bugs.

Conclusion

While this list is nowhere near exhaustive, it can be an excellent first step in improving the quality of code reviews in a team. Of course, it also goes without saying that reviewers need to have compassion and empathy when doing a review. At the end of the day, the idea is to grow as a team and work more efficiently together in peace.