Photo by Austin Distel on Unsplash

10 Proven Techniques for More Effective Code Reviews

Introduction

As much as we know that code reviews are mandatory, the jury is out about how best to dish them out. A code review becomes an intersection of human and machine collaboration. Humans debate and collaborate to better the machine code to ensure its intended function.

It’s this very intersection where the problem begins. Humans have a hard enough time conversing with each other, never mind talking about code that a machine is barely intelligent enough to process. Often, code reviews can become personal, offensive and filled with dread. They can hold up sprints or project deadlines due to stubborn and egotistical mindsets or cause panic in those who don’t understand the intended feedback.

Fortunately, there are ways to overcome these challenges, so here are ten techniques for giving, accepting and working with code reviews:

Perform Multiple Reviews Per Feature

This helps you, as the reviewer, understand what the engineer submitting the code review is trying to achieve; it outlines their thought process and uncovers their struggles.

I suggest regular check-ins regarding code reviews, perhaps when a feature is 25% or 75% complete. This gives you multiple opportunities to course correct if need be. Junior developers will also appreciate the guidance, as it will feel like you’re building the feature with them.

As time may not always allow for three rounds of detailed review, doing a brief check-in via a Draft PR for the 25% and 75% milestones may be worthwhile. A thorough review can be performed when the submitting engineer thinks the feature is finished.

Define a Clear Process

  • Who will perform code reviews?
  • What automated tools will be used?
  • How often should a code review take place?
  • What activities will take place during the code review?
  • What are the steps to submit a code review?
  • Who is responsible for assigning tickets/code reviews to relevant engineers?
  • How should developers prepare their branches?
  • What merge strategy should be used?
  • Should branches be deleted after merging?

It often helps to define a document that outlines these questions and their answers to help make the team more efficient. Once all members agree on a process, follow it and try to keep it the same.

Always communicate with the team when there’s a process change and ensure complete alignment and understanding.

Focus On Code, Not People

Feedback like this can often come back as “too direct” and does not contribute to a cohesive working environment. When addressing feedback on a code review, try to:

  1. Keep it light-hearted, kid around a bit, or use emojis.
  2. Be empathetic.
  3. Suggest, rather than instruct: “Perhaps it would be better to…” rather than “You must stop doing….”
  4. Avoid blameful or aggravating language: “A better understanding of the CSS cascade would help make this code more efficient” instead of “You don’t understand how the CSS cascade works”.
  5. Treat every feedback item as a learning experience for the engineer having their work reviewed. Likewise, as the code reviewer, every feedback item you leave is a teaching moment.

Ask Questions; Dont Assume Mistakes

  1. “Can you explain to me how this function works?”
  2. “Why is this value a String on line 117 but an Array on 120?”
  3. “What is the purpose of this variable?”

Asking questions disarms the engineer who submitted the request and helps you understand the logic better. The reviewee will feel like they are teaching you, increasing their self-confidence.

Leverage Automated Tooling

Many platforms offer integrations for automated tooling to run in CI/CD pipelines that can make this a breeze. Github, for instance, allows you to run automated tasks through GitHub Actions, which can fire on pull requests or git push.

You can also use tools like Husky or Lint-Staged to run linting scripts on only the code that has changed on the branch. This way, code styling, best practices and silly errors can be caught without needing a human eye.

Of course, automated tools can’t catch everything, but I would consider them for the following:

  1. CSS Code Styles
  2. JavaScript Code Styles
  3. Framework Best Practices
  4. Unit Testing
  5. Performance / e2e Testing

Encourage Open Communication and Dialogue

While having these conversations, it’s essential to gauge the following:

  1. How much time you’re spending mentoring, teaching and explaining the issues found in the PR?
  2. The openness of the reviewee. They also have to take the code review process as a learning experience and avoid getting too conscious of the feedback provided.

Actively encourage the challenging of opinion in a way that keeps open dialogue consistent. Less senior team members need to be encouraged to challenge opinions respectfully. This can be done in a 1:1 session or as a team feedback session if the reviewee feels comfortable.

Do More Than Review

  1. Pulling the code down locally and running a production build.
  2. Testing the feature in the browser, validating the technical approach and ensuring it meets product stakeholder expectations.
  3. Performing some light but meaningful QA on the feature.
  4. Suggesting improvements and providing options for optimising.
  5. Thinking ahead and determining if this feature could have a domino effect across the project.

Be aware of these extra responsibilities. As the reviewing engineer, you give the thumbs up for when this code is merged.

Use Code Suggestions and Examples

In the past, I’ve received feedback that I haven’t entirely understood or that’s made me hopelessly lost. Sometimes the feedback required a subtle refactor or update to logic I did not completely understand.

If a reviewing engineer had provided a clear example of what they wanted to see, I could have been far more efficient in my duties. GitHub has a great feature called Suggested Changes, allowing reviewers to suggest code updates in line with commented feedback.

Linking to related examples, educational reading, articles or blog posts can also help achieve positive and sought-after results from reviewees in a way that teaches them and allows them to improve.

Don’t Sweat The Small Stuff

As a senior engineer on the project, you have the authority to override this. However, it does become a case of “choosing your battles”. It’s not efficient nor practical to have back and forth on how a constant has been named.

You need to learn to let certain things go.

This can be dependent on the technologies or frameworks used. As a general rule of thumb, always prioritise the issues that could cause the most damage in the long run. Naming conventions and other callouts can be tackled later if they do not directly affect the functionality or cause code styles to erode on lint.

Implement a Code Review Check List

For other more abstract items, though, a checklist in the PR description can go a long way. You may want to include the following:

  1. A link to the task ticket.
  2. A description of the technical approach or solution.
  3. Links to designs.
  4. Screenshots of the feature.
  5. Instructions on how to test.
  6. A section to confirm functionality, address doubts etc.

If every engineer takes the time to fill in this section consistently, it provides a meaningful database of progress over time. This can be useful when uncovering where a bug originated. In GitHub, you can use PR Templates to implement a pre-defined setup when a PR is created.

Conclusion

Remember to communicate openly, provide direct feedback on the code, not the person, perform thorough reviews beyond “just reviewing code”, and leverage automated tooling to save you from searching for remedial mistakes.

This article originally appeared on dainemawer.com. If you liked this article, please follow me on Twitter at @daine_mawer.

--

--

Associate Director of Front-end Engineering at 10up.com | Writer | Traveller | Surfer

Get the Medium app

A button that says 'Download on the App Store', and if clicked it will lead you to the iOS App store
A button that says 'Get it on, Google Play', and if clicked it will lead you to the Google Play store
Daine Mawer

Associate Director of Front-end Engineering at 10up.com | Writer | Traveller | Surfer