Healthy approach to code reviews

Code reviews are weird monsters. With no doubts they improve the overall quality of the code and help programmers build relationships and work together more effectively.

However asking for a code review puts many people on edge because it is just like laying your creative soul bare. By generally speaking most of the devs lives CRs as a “personal intrusion”.

Your team is only good as your weakest reviewer

Here are some tips I’m collecting about it in these years.


Code Review is a team activity. A good mixture of senior and junior members aim to create the right environment for learning: senior members can battle test their code related to APIs clarity, junior devs have a chance to learn new things.

Depending your team size and structure each review may require one or more engineering approval to be merged. From my experience if your team is pretty small (less than 5 people) you can stick with 2 reviewers to keep a good balance on sharing knowledge.

If you are growing over 8+ (without moving to cross-functional teams) you should change your approach: your CR should require at least one engineer and you should start moving the team to a more static organization where part of the program are assigned to an ownership which is also responsible for manteniance and evolution of that code (more specialized skills, less shared knowledge with a modular architecture which follow a shared vision – we’ll look at that in one of the upcoming posts).

Depending of the affected part you may decide to assign it to more people; I’ve seen it vary, for core layers you may considering sharing the knowledge across the entire team.

Help me, help you

We don’t have hard requirements for adding descriptions to pull requests but recently I found it very useful and instructive to use Templates. It help you to create a good habit both for the author and any reviewer.

First of all the author is allowed to take a deep breath and focus on what he did, describing the idea, the implementation and force himself to take an overview look to the PR.

That said, anyone who is reviewing code will need to figure out what is changing and why it’s changing.

In general, we think it’s good to consider what people may or may not know about your PR, and aim to give them context so that they can help you.

Small PRs is not a matter of LoC

I strive to build things in small units; when planning a new milestone my approach is the old divide et impera. I want to break our code down as much as possible and submit small, digestible PRs. By doing this, the team can review quicker and move faster.

However, when you think about a small PR you often refer in terms of lines of code (LoC). Instead you should think in terms of conceptual changes.

For example, a variable name refactor maybe a big review because it may impacts lots of files in your code base. However it still small because it’s just a single concept (the renaming).

In my engineering we values and encourages the combination of small PRs and quick reviews.

Author POV

As author of a merge request is your responsibility to avoid waste of time & energy of your collegues.

CR must be:

Reviewer POV

Your work as reviewer is only partially related to the tech side; you should put yourself into the correct mindset when approaching a new merge request.

What you should check in a Code Review

When you are doing a CR you should focus to the following list of activities:


The reason a code exists is to satisfy one or more business requirements.


Think carefully about how you would have solved the same problem.


When possible code must require a low cognitive load to be understood. Over-engineering is a particular type of complexity where developers have made the code more generic than it needs to be, or added functionality that isn’t presently needed by the system.

As reviewer you should be especially vigilant about over-engineering to limiting future speculation.

Naming & Conventions

Naming and conventions are tools to reduce cognitive load. As team you should create, maintain and update your list of conventions; this set of rules does not strictly depend by technologies but also by the single personality of the team members.

By generally speaking you should keep in mind the following questions:

Tests & Docs

If your team maintain a test suits, as reviewer always ask for unit, integration, or end-to-end tests as appropriate for the change. Tests should be part of the code review unless author is handling an emergency.

Key Takeaways

Keep in mind: code review is an asynchrnous activity don’t bother your reviewers with real-time communication unless you can’t come to an agreement or you think it’s necessary to the complexity of the changes.

Internet is full of articles about this topic; the links below are a short list of further readings where other details are discussed.

· leadership