How to do a code review — some tips and tricks from the Productboard engineering team

How to do a code review — some tips and tricks from the Productboard engineering team

The code review is an essential step in shipping high-quality code. What’s more, it allows developers to share knowledge, give feedback, and learn from each other’s expertise and mistakes. But depending on how we do code review, it can either be a time-consuming hassle or a breeze. 

In this article, I’ll share some tips that we use here at Productboard to ensure that code review runs smoothly — for both authors and reviewers. Let’s start at the beginning.

Pull requests are a dialogue, so start them politely

I believe that pull requests (PRs) should be seen as a dialogue between the author and the reviewer. By opening the PR, the author is effectively kicking off the conversation. 

For this conversation to be productive and positive, the author must start the dialogue politely. If they don’t — if they are rude or disrespectful — the other person in the conversation probably won’t want to continue. So it’s critical that we get off on the right foot. In my experience, there are five key steps we can take to ensure that we are mindful PR authors:

  1. Start with your draft and wait for continuous integration (CI)
  2. Provide context
  3. Conduct a self-review
  4. Annotate parts that may be unclear
  5. Get approval from your team first

Let’s look at each step in more detail. 

1. Start with your draft and wait for CI

When we create the PR and open it in the draft, we make sure that we don’t send notifications to everyone that’s listed as a co-owner in the PR. We don’t want to create unnecessary noise for reviewers by sending them a link to a PR that isn’t ready yet.

Before we bother the reviewer, we want to ensure that our code has passed CI. Asking someone to review a PR that’s failing automated tests is disrespectful of their time.

2. Provide context

Next up is perhaps the most important step in a successful author-reviewer dialogue: providing context. By providing context, we fill in the blanks, giving the reviewer the information they need to fully understand the code and the purpose behind it. Without context, the reviewer is left with questions but no answers. 

If you fail to provide context with your PR, you might get lucky with a patient reviewer who’s happy to calmly ask you to provide more information. But even so, chances are you waited a while for the reviewer to get to your PR. Now you’ll have to go to the back of the line once again and wait even longer. This slows the whole process down, making it needlessly frustrating not only for the review but for you as well. 

If you aren’t lucky enough to get a patient and diligent reviewer, things could be even worse. Instead of asking for that much-needed context, the reviewer might blindly approve your PR. Now, this is really bad. The goal of a PR is to get a code review, not an approval. If a reviewer doesn’t fully understand the motivation behind the PR but approves it anyway, you haven’t fulfilled the purpose of the PR.

So what does context mean in practice? Glad you asked. In my experience, providing the following information ensures that the reviewer has everything they need to conduct an effective code review:

  • A PR title containing a Jira ticket number, a verb, and a subject — this provides the reviewer with context around what is being proposed
  • A PR description answering two questions: Why have you created the PR, and what are you trying to achieve through it?

3. Conduct a self-review

Next is the important process of self-reviewing your PR. This is where you, the PR author, attempt to look at your own code through the reviewer’s eyes, before sending it to the actual reviewer. This process helps us to find mistakes we may have missed initially, such as removing console.logs, or to clean up the PR by removing unused code. 

If we don’t carry out a self-review, we’re passing the responsibility for finding and commenting on those silly mistakes to the reviewer, which shows a lack of respect for their time. We should always be mindful of the reviewer’s time and do whatever we can to make their job as easy as possible.

4. Annotate parts that may be unclear

During the self-review process, we’ll also be able to spot parts of the code that may be unclear to the reviewer. We know everything about our code because we wrote it. But the reviewer doesn’t have all the context we have, even if we provide some in the PR description. 

By proactively annotating parts of the code that would potentially be unclear for the reviewer, we help them to understand why we wrote the code that way. This saves all that wasted time and energy spent on back-and-forth questions and answers. 

Where possible, it’s best to add annotations as comments in the code. This ensures that the comments stick with the code and are there for everyone to see. But this approach is not always possible — if you are using JSON, for example. In these cases, you can annotate the code in the PR itself, like in the example below. 

5. Get approval from your team first

Now, we’re almost ready to make the PR ready for review. But first, there’s one more step to complete to be a truly mindful PR author. Before we ask other teams to look at our PR, we want to get a consensus on it from our own team. By using your team as a final filter, you minimize the noise and effort for other teams. 

I understand that in many companies, code reviews are carried out within the developer’s team as standard. In that case, carry on! But in Productboard, we use ‘codeowners’ in Github, which allows us to ask for a review from every team that owns at least one file in the PR.

Now our PR is ready for review!

Once we get the green light from our team, we can go ahead and make the PR ready for review. By the time it gets to the reviewer, our PR will be clean, concise, and full of context. I truly believe that if we all follow this simple guide, we’ll not only be happier developers, but we’ll also be far more productive. 

That’s all from me on this subject. I hope you found these tips useful. Now, I’d like to throw it back to you. How do you approach code reviews? What tips do you have for making them more efficient and effective?

 

Fancy joining us on our mission to make products that matter? We’re hiring across the board. Head over to our careers page to see our available positions. We’d love to hear from you!

You might also like

Productboard expanding San Francisco engineering presence
Life at Productboard

Productboard expanding San Francisco engineering presence

Jiri Necas
Jiri Necas
Refactoring Productboard’s design system to the next level
Life at Productboard

Refactoring Productboard’s design system to the next level

Jonathan Atkins
Jonathan Atkins
The Role of Growth Engineering at Productboard: Significance, key skills, and responsibilities
Life at Productboard

The Role of Growth Engineering at Productboard: Significance, key skills, and responsibilities

Stuart Cavill
Stuart Cavill