DEV Community

Nicolas Bonnici
Nicolas Bonnici

Posted on • Edited on

Efficient code review process

Welcome. If you're here reading this post, it means you already contributed to some projects where you were not alone, or you plan to. If you want to know more about what we call code review (aka code contribution reviewed by peers before merging it into a common trunk), read on.

Mindset

In this process, the main purpose is to challenge a technical response to a problem within the project context.

If a reviewer suggests another approach, you must ask yourself: is it a better approach than yours? If the project can benefit from the suggested change in any domain—user experience, developer experience, security, performance, or code clarity—you must consider it seriously and push the requested changes. Otherwise, at least ask for more information and, if needed, engage in a direct discussion.

The whole process should be taken as nothing but a win-win. The more care you put into the review process, the more the produced code quality will improve, and thus the overall project quality will benefit as well.

Code hygiene and stability are the main goals. You guarantee best practices, security, maintainability, and performance. Sometimes, on some projects, this is not already the case and you have to maintain a legacy project anyway. Apply strictly the same process to all new contributions; then you can leverage a technical roadmap to clean what needs to be cleaned.

Of course, but it’s important to mention again: be respectful, kind, and never get upset—unless you're Linus Torvalds. Be open to suggested changes.

Interact with the reviewed developer

Using the most popular Git web interfaces such as GitHub or GitLab, there are many ways to interact with the code author. Sometimes the changes you request are not mandatory or can be postponed on a technical roadmap. Sometimes the contribution will directly alter the project's code quality, security, or performance at merge time, so they must be addressed beforehand.

When you open a thread and ask the author to implicitly push some changes, a suggestion is always easier to understand than a long explanation without one.

It's your responsibility to follow up and resolve threads as soon as possible to avoid blocking the next steps in the process. Using common web apps such as GitHub or GitLab, you have administration panels where all your pending reviews can be found. Be sure to use those pages: on GitLab it's https://gitlab.com/dashboard/todos and on GitHub https://github.com/pulls/review-requested.

Sometimes the best way to understand each other is to simply start talking in person or in a one-to-one online chat.

Project size considerations

The more contributors your project has, the stricter the review and merge process must be. Be careful not to be overkill, but also not to be too permissive.

At least 20% positive reviews from all contributors, combined with robust CI steps such as code audits, linting, and tests, should be sufficient. You can leverage role concepts in Git web apps to ensure at least one core maintainer approval for projects with many contributors.

Again, your process, your rules—just make sure they are documented somewhere. Pull/merge request title formats, description metadata, and any other rules are the foundation of this process.

The process

First of all, the smaller the contributions are, the better this process works. Avoid TLTR reviews (Too Long To Read) by splitting task scopes into small chunks.

Checklist

  • First of all, if you take time to review a merge request, the developer who assigned you must deliver a stable branch and a finished contribution. All CI and code quality steps must be green.

  • Another important point is to check that the reviewed code is correctly rebased on top of the target branch HEAD and contains no conflicts at all. Otherwise, ask the contributor to rebase their branch before reviewing the code.

  • The code lint must be valid, all kinds of test suites must pass, and all code quality analysis tools must succeed. Otherwise, ask the developer for related changes before reviewing the code.

  • Now it's time to analyze the changes: which files were edited or added? You need a global point of view first. The more context the developer adds—such as explanations, links to tickets, or even media like videos or screen captures—the better the reviewer will understand the purpose of the code. Document or override Git commit templates to set the standards your team needs.

  • Does the code add a new feature or modify an existing one? Is it a fix? In any case, the code should be backed by tests to ensure it works as expected, fails correctly, and does not regress later.

  • Is this contribution sufficiently documented? Is the specification properly updated?

  • Does the contribution properly handle errors or anything that could break normal code behavior? Is it possible to understand application behavior through logs?

  • Can this contribution be simplified in any way?

  • Is this contribution at the right abstraction level?

  • Does this project have solid code analysis steps? Is the test coverage at least 80%, and are critical user stories tested end to end? If not, you must check out the branch locally and rebuild it from scratch. Ensure all available tools for testing, quality, and linting are used. A clean CI/CD pipeline is the core of a clean project.

  • Does this code alter the database or persistence layer? Is the migration process safe when the CD pipeline applies it to production with existing large datasets?

  • Can this contribution expose unwanted sensitive data?

  • Does this code alter global project performance? Is there a risk with high data volumes? Are new database indexes needed? Will it scale?

  • Is this contribution safe, or does it introduce a new security flaw? Bad practices, potential XSS vulnerabilities, unsafe secrets committed to Git, or backdoors introduced by malicious contributors. Security tools should be part of your CI pipeline.

  • Does the project have contribution rules (often in a CONTRIBUTING.md file)? Does this contribution respect them?

Do and don't

Do

  • Respect the checklist above and review code only when the contributor considers it ready.

  • Take the time to review thoroughly.

  • Ask the developer to flag their contribution as WIP or draft if the code is not ready.

  • Add context to your review threads. Sometimes your request is not obvious. Saying something is “cleaner” or “better” without explanation is not helpful. Example: “Can you split this service into two—one in domain X and one in domain Y—to avoid mixing responsibilities?”

  • Ask for small contributions. Split tasks into small chunks to avoid TLTR reviews.

  • Always assign the best possible reviewers. Find experts for each part of the stack you touch.

Don't

  • Assume a developer is so senior that you can approve blindly. No one is perfect—even AI makes mistakes. Doing so defeats the purpose of code review.

  • Review WIP code. Such contributions should be marked as drafts.

  • Approve untested contributions.

  • Approve commented-out code.

  • Approve unused code.

  • Approve something you don’t understand.

  • Approve hacks or workarounds instead of proper fixes.

  • Repeat yourself for typos. If a typo appears everywhere, point it out once instead of opening dozens of threads. It wastes time and helps no one.

Gatekeeping

One phenomenon you will encounter is known as gatekeeping—in other words, someone being extremely strict about contributions, or wanting everyone to code exactly the way they do.

The best way to deal with gatekeeping is to define strict rules and document them, especially around contributions.

You can also improve developer experience with code quality rules, templates, linting, and automated checks directly in your CI process.

Most of the time, gatekeepers are more about rigid adherence to project rules. If it concerns project rules rather than personal preferences, thank that reviewer—they are fulfilling the core purpose of this process.

Word of end

Keep in mind that code reviews are one of the best and fastest ways to grow as a developer. A single thoughtful comment from a generous reviewer can significantly improve your skills.

Thank you for reading. Feel free to share your own best practices for reviewing code in the comments below.

Top comments (0)