How To Do Code Reviews Properly

Dr Milan Milanović
6 min readDec 20, 2023

Code review is an essential step in the software development lifecycle. It enables developers to significantly enhance code quality. It resembles the authoring of a book. First, the author writes the story, which is edited to ensure no mistakes, like mixing up “you’re” with “yours.” Code review, in this context, refers to examining and assessing other people’s code. It is based on the pull request model, popularized by open source.

A code review has different benefits:

  • It ensures consistency in design and implementation,
  • Optimizes code for better performance,
  • It is an opportunity to learn,
  • Knowledge sharing and mentoring, and
  • Promotes team cohesion.

What To Check In Code Reviews?

A code review has different benefits:

What should you look for in a code review? There are different things that we could check, from the different levels of importance and possibility of automation:

Try to look for things such as:

  1. Functionality & Design—Here, we need to answer questions such as: Does this change follow our desired architecture or integrate well with the rest of the system? Does it have high cohesion and low coupling between components? Does it follow sound principles like OO, SOLID, DRY, KISS, YAGNI, and more?
  2. Implementation—Here, we check if the solution is logically correct (it does change what the developer intended), if this code is more complex than it should be, etc. Is the code needed? We also check whether we use good design patterns and the critical stuff, such as the API entry point.
  3. Testing—Here, we check whether all tests pass, whether we use unit tests for all code paths and behaviors, and whether we use integration tests for external systems (DBs, file systems, etc.). Do we check all edge conditions and code coverage from 60–80%?
  4. Documentation—Has our PR description been added? Is our solution documented in the README.md in the repo or elsewhere updated?
  5. Code Styles — Do we follow our project code styles? Do you know if naming is good? Did the developer choose exact names for classes, methods, etc.? Is the code readable?
Code Review Pyramid (based on the original work of Gunnar Morling)

Some Good Practices When Doing a Code Review

Here are some good practices when doing a code review:

  1. Try to review your code first.

Before sending the code to your colleagues, try to read and understand it first. Search for parts that confuse you. Seeing your code outside an IDE often helps you view it as “new” and avoid operational blindness.

2. Write a short description of what has changed.

This should explain what changes were at a high level and why those changes were made.

3. Automate what can be automated.

Leave everything that can be automated to the system, such as checking for successful builds (CI), style changes (linters), automated tests, and static code analysis (e.g., with SonarQube). We have integrated it on the PR level, so it will run on every PR and give blockers for code merge. This will enable us to remove unnecessary discussions and leave room for more important ones.

4. Do a kick-off for more significant stories with your team members.

If you are starting to work on something more significant, especially design-wise, try first to do a kick-off with the code owner or someone who will be your code reviewer. This will enable an agreement before the implementation and reduce the effort on the PR review level with no surprises.

5. Don’t rush

You need to understand what has changed around it—every line. Read multiple times if required, class by class. One should look at every line of code assigned for review. Some codes need more thought and some less, but we must make a decision during a review. Make yourself available for verbal discussion if you need to.

6. Comment with kindness

Never mention the person (you), always focus on changes as questions or suggestions, and leave at least one positive comment. Explain the “why” in your words and advise on how to improve it.

7. Approve PR when it’s good enough.

Don’t strive for perfection, but hold to high standards. Don’t be a nitpicker.

8. Make reviews manageable in size.

We should limit the number of lines of code for review in one sitting. PR should contain as few changed files as possible. I prefer more minor incremental changes to significant sweeping changes. Our brains cannot process so much information at once. The ideal number of LOC is 200 to 400 lines of the core at one time, usually 60 to 90 minutes. If you have an enormous task, refine it into smaller sub-tasks that can be quickly reviewed.

9. Use checklists

Another thing you can do here is to have a tool that can be used to go through all aspects of a code review before adding reviewers. We use pull request templates in Markdown on Azure DevOps, which are applied to the description field when a pull request is created. E.g.

Thank you for your contribution to this repo. Before submitting this PR, please make sure the:

- [ ] Your code builds clean without any errors or warnings
- [ ] You are using the appropriate design
- [ ] You have added unit tests and are all green

These templates could be different for a branch or can be optional, too.

10. Use tools

For all code reviews, you should use some tools, such as BitBucket, Azure DevOps, GitHub, or GitLab. For example, Microsoft has used an internal tool called CodeFlow for years, which supports developers and guides them through all code review steps. It helps during the preparation of the code, automatically notifies reviewers, and has a rich commenting and discussion functionality. In the later years, they switched to GitHub Pull Requests. Google is also using two kinds of solutions for code reviews. They use the Gerrit code review tool for open-source code, yet they use an internal tool called Critique for internal code.

A Better Alternative To Code Reviews

Another alternative to classic PR reviews could help you gain more efficiency and speed in your coding process. It is based on a model other than Pull Request called Trunk-based development. Here, you synchronously have code reviews. In this way, all developers work on the mainline branch, frequently committing to it. An example of such practice is a Collaborative programming approach (Pair and Mob programming), introduced as an Extreme Programming technique by Kent Beck in the ‘90s.

Pair programming (Credits: Unsplash)

Pair programming and mob programming are collaborative programming approaches that involve two or more developers working together on a single task, sharing ideas and thoughts while writing code. In these approaches, one developer acts as a driver (writes code), while the other plays a navigator role (ensures code accuracy). They switch positions from time to time during the process.

These methods offer several benefits compared to classic code reviews:

  • Real-time feedback: Pair and mob programming allow developers to receive immediate feedback on their code, enabling them to address issues and improve. This contrasts with classic code reviews, where feedback might be delayed until the code review stage.
  • Enhanced knowledge sharing: Collaborative programming enables developers to learn from each other’s experience and knowledge, leading to better code and skill development. This is particularly helpful when working with new technologies or for newbies. Learning is also more easily spread among team members, reducing the risk of a single developer becoming a bottleneck.
  • Faster problem-solving encourages developers to work together to solve problems, leading to quicker and more efficient solutions. This can reduce development time and improve project outcomes.
  • Increased focus and productivity: Working closely with another developer can help to maintain focus and reduce distractions.
  • Improved code quality: Multiple developers working together are more likely to catch errors and design issues early in development.

This approach works best when you have a co-located team of mostly senior developers who must iterate fast. However, the Pull Request Model works better if you have a team of juniors or have a more complex product where more than one person needs to review the code.

Read the rest of the article, including bonus chapters in the Tech World With Milan Newsletter.

To expand your knowledge and personal growth, subscribe to my free weekly newsletter with 20,000+ people: https://newsletter.techworld-with-milan.com.

Originally published at https://newsletter.techworld-with-milan.com on March 30, 2023.

--

--