DEV Community

Cover image for 16 best practices to make your code reviews better
Kimmo Brunfeldt
Kimmo Brunfeldt

Posted on • Originally published at swarmia.com

16 best practices to make your code reviews better

Code reviews are a widely accepted best practice for software development teams. In this post, we'll cover why the most successful teams use code reviews, how to adopt them in your development process, and what the best practices are.

The goals of code reviews are:

  • Sharing knowledge: The depth of know-how shared depends on the thoroughness of the review, but some amount of information will always be transferred. The knowledge can be general tips about the framework or programming language or invaluable domain-specific information bits.
  • Spreading ownership: Code reviews have a positive impact on mutual code ownership. It's easy to end up in a situation where one developer always deals with a certain part of the codebase because they're most familiar with it. It might be a short-term win but is often a long-term loss. When ownership is shared, teams become more motivated and autonomous.
  • Unifying development practices: Every developer has their own tendencies and ways to implement software. Code reviews help to narrow the gap between individual development styles and make the codebase more unified. Unification happens through high-level discussions about architecture and software design and via micro-level continuous integration checks, such as coding style enforcement.
  • Quality control: Studies have shown that code reviews can help with catching defects, but even more importantly, they surface software design issues while they are still relatively easy to change.

The four whys of code reviews

Adopting code reviews

It's crucial to set the review process right. At worst, code reviews might feel like a hindrance. At best, code reviews help to sustain a good, stable team performance for many years.

If your organization is new to code reviews, introducing them will be a big change in the development process. Whenever implementing changes to ways of working, it's a good idea to ensure that everyone agrees on the process and has had the chance to contribute to the decision. This will cause less friction.

You, as a team or an organization, should agree on the philosophy and motivation behind code reviews before implementing them. You can write down an internal "what's a good code review" document together or refer to existing guides. It's a practical way to make sure everyone is aware of the whys.

Social relationships can't be ignored when talking about peers giving feedback about each other’s work. There are no silver bullets. It's hard work that requires each individual's contribution. To have the best chance of success, make sure to thoroughly discuss and educate your team about communication practices.

Best practices for code reviews

Blog posts about review practices often mention smaller details about branch names, commit message lengths, etc. While those tips are valuable too, this post focuses on more general recommendations.

There are multiple perspectives to a code review process: the author's, the reviewer's, and the team's point of view. Each party has an equally important role in the process. Some best practices apply only to the author or the reviewer, but many of them are important for everyone in the team.

Let's go through what those practices are. We'll focus on GitHub and pull request (PR) oriented review processes, but many of the tips apply in general as well.

1. Decide on a process

Responsibility will bounce between the author and the reviewer(s). The more explicit the review process, the less likely the ball is dropped by any party.

For example, our internal PR guidelines look something like this:

  1. DRAFT You can open a PR in a draft state to show progress or request early feedback.
  2. READY FOR REVIEW You can also skip the draft state and open a PR directly.
    • The review is done by another team member.
    • Usually, there's no need to request a review; Swarmia automatically notifies the team. A manual review request can be useful if you, for example, want to request a design review from a certain designer.
    • It's polite to have the PR ready so that you're not about to rebase everything 5 times while the reviewer tries to keep up.
  3. CHANGES REQUESTED Fix the requested changes, or discuss whether a fix is needed.
    • Preferably, create new commits after the review.
    • You can directly commit suggestions from the GitHub UI.
  4. APPROVED The author is responsible for merging their own PR.

Sketch of our internal process

Yours might look different, but as long as the team agrees on the process, all is good.

2. Focus on the right things

To maintain code review standards across developers, it's a good idea to have guidelines for what to focus on in code reviews. Here's what we recommend focusing on:

  • Functionality: Does the code behave as the PR author likely intended? Does the code behave as users would expect?
  • Software design: Is the code well-designed and fitted to the surrounding architecture?
  • Complexity: Would another developer be able to easily understand and use the code?
  • Tests: Does the PR have correct and well-designed automated tests?
  • Naming: Are names for variables, functions, etc. descriptive?
  • Comments: Are the comments clear and useful?
  • Documentation: Did the author also update relevant documentation?

Developers shouldn't spend their time reviewing things that can be automatically checked. More on that in "Use continuous integration effectively" and "Delegate nit-picking to a computer".

3. Discuss the high-level approach early

Before jumping into coding a complex feature, it's beneficial to discuss the high-level approach first. Usually, this is done when planning the feature.

It's not nice for anyone if a PR ends up in a complete rewrite because the approach wasn't discussed beforehand. Rewrites of pull requests do happen every once in a while, but it's a sign that you might want to talk more before the implementation.

Sometimes a proof-of-concept implementation is needed to ignite the discussion. An effective way to get started is to open a draft PR of the approach and make the architecture decision based on the gained information.

4. Optimize for the team

This idea is explained well in Google's Engineering Practices document:

We optimize for the speed at which a team of developers can produce a product together, as opposed to optimizing for the speed at which an individual developer can write code.

Speedy reviews increase team performance in multiple ways: iteration becomes faster, developers don't need to do time-costly context switches as often, etc. Make sure the team understands the implications of fast reviews and agrees on a suitable maximum time for responding to a PR. The key is to minimize the response lag between the author and the reviewer, even if the whole review process takes long. For example, it might be invaluable for the author to know that their PR will be reviewed, for example, tomorrow morning.

That said, developers shouldn't interrupt their focus to do reviews. Instead, they should prioritize them whenever there's a fitting gap—for example after lunch.

Review speed is definitely not solely the reviewer's responsibility—the author has an important role too. The easier it is to pick a pull request and review it, the faster the work flows. Our help article "Review Code Faster" covers how to leverage Swarmia to speed up reviews.

5. Default to action

Sometimes reviews can stall for various reasons. During those times, you should have a bias for action. One can approve a PR even if there's some input left for the author to consider.

If a tech decision lingers and work becomes blocked, deciding something relatively quickly is better than slowly concluding to an "ideal" decision. Reserve enough time for technical decisions, but move on before you reach analysis paralysis. Developers should be inclined to merge code instead of primarily focusing on poking holes in the implementation.

Ship it!

6. Keep pull requests small

Smaller batches are easier to design, test, review, and merge. There are studies [1] [2] about the optimal amount of changed lines, but it's not an exact science. Google's recommendations put it well:

There are no hard and fast rules about how large is “too large.” 100 lines is usually a reasonable size for a CL, and 1000 lines is usually too large, but it’s up to the judgment of your reviewer. The number of files that a change is spread across also affects its “size.” A 200-line change in one file might be okay, but spread across 50 files it would usually be too large.

It's almost always possible to split a large change into smaller chunks—for example, with a separate refactoring PR that sets the stage for a cleaner implementation. Practicing slicing also helps to detect minimal shippable increments of your product.

Feature gates or feature flags might be necessary to gain the ability to ship half-ready product features along with the existing ones.

7. Foster a positive feedback culture

Effective communication, in general, is really hard. Giving feedback about a colleague's work is one of the most challenging forms of communication. Acknowledge this in code reviews.

Here's a list of suggestions to improve discussions in code reviews:

  • Give feedback about the code, not about the author.
  • Pick your battles.
  • Accept that there are multiple correct solutions to a problem.
  • You're in the same boat.
  • PR authors are humans with feelings (except dependabot 🤖).
  • Provide reasons, not feelings, to support your position.
  • Use the "Yes, and..." technique to keep an innovative atmosphere. It can be an ungracious pattern to dismiss fresh and fragile ideas in a draft PR stage.
  • Keep the feedback balanced with positive comments. It's always delightful to receive praise from the reviewer.

If the pull request discussion becomes heated, schedule a call to discuss the topic. It usually helps to relieve the tension.

8. Use continuous integration effectively

GitHub Actions and status checks are widely used for building a robust CI pipeline. However, no matter the tools, it's crucial to invest in setting up a CI solution to automate as many quality checks as possible.

Automated checks allow reviewers to focus on more important topics such as software design, architecture, and readability. Checks can include tests, test coverage, code style enforcements, commit message conventions, static analysis, and much more.

A variety of metrics produced by continuous integration can help the reviewer to quantify the quality of a PR. Test coverage and code complexity metrics might reveal interesting insights that otherwise would be hard to estimate. These metrics don't necessarily need to be hard pass or fail checks but rather additional data for the review process.

Instead of slowing down the review process to catch more bugs, try to improve the automated checks to enable fast movement.

GitHub pull request checks for Swarmia's frontend repository

9. Delegate nit-picking to a computer

Whenever a reviewer spends their time nit-picking on small details, consider if it could be an automated check. Automatic check will always be enforced, while the human process relies on the reviewers' memories and moods to reject an anti-pattern.

For example, our ESLint rules enforce a consistent usage of certain terminology across the product. This is far more effective than documentation that would list the correct spelling of each word.

Code formatting is an example of a controversial topic in which almost all solutions are correct. Spending time debating stylistic choices rarely provides much value to the product, as long as a set of rules are adopted. Consistent unpleasing styles (to some individuals) are better than a mixture of multiple styles.

Your team can also adopt pre-existing practices, for example, a TypeScript project could adopt Prettier defaults instead of re-inventing their own.

10. Communicate explicitly

When reviewing a piece of code, be explicit about the action you request from the author. Let's say a reviewer has commented, "This could be done in Postgres in favor of application code" on a line of code. Are they requesting a change, suggesting to refactor it later, or just making the author aware of other solutions? It's often hard to judge. GitHub provides tools to be more explicit: for example, "Request changes" in a review.

Tip for the PR author: dismissing a review resets the pull request state to indicate that the reviewer can review again. It's up to you, the PR author, to decide if it feels important enough to use the feature, but especially in remote teams, it might help to make the process even more explicit.

11. Use explicit review requests

Review requests in GitHub are a convenient way to let others know that your code is ready for review. While a review can be requested manually, we recommend setting up a CODEOWNERS file to automate requesting a review. The method is robust and doesn't rely on individual authors remembering to request reviews.

Reviews can also be requested from a team, via CODEOWNERS or manually, which distributes the responsibility among team members. Make sure reviews are done evenly by all developers instead of siloing reviews to a single person.

Example of a CODEOWNERS file from Swarmia's frontend repository telling GitHub to notify @swarmia/engineers team whenever a new PR is opened

Example of an email sent from a PR based on a CODEOWNERS file

12. Review your own code

Before submitting a PR for a review, go through the changes yourself. This helps to catch accidentally included changes, typos, and other simple mistakes that potentially waste the reviewer's time.

13. Document as much as possible in code

When receiving a comment or suggestion, aim for documenting the discussion in code. If the reviewer is not sure what the validateUsers function does, elaborate on the functionality ideally by renaming the function or writing a comment in the code. This way, the next developer that reads the code will understand the functionality without reading the PR discussions.

In some cases, the author can copy-paste their PR discussion response as is to comment in the code.

14. Write clear PR descriptions

The reviewer forms a mental picture of a pull request from multiple information sources: feature planning, description in the issue tracker, PR description, commit messages, chat history, etc. The more coherent the picture is, the faster and higher quality the review is. Decide on the team’s preferred channels to communicate certain information.

At Swarmia, we use PR descriptions to fill the technical gaps that the Jira issue description didn't cover. The additional details often include information such as what setup is needed to test the PR, surprising implementation details, and anything that makes the reviewer's job smoother. There are also other ways to add information: code comments, commit messages, commenting on individual lines in a PR as the author, etc.

Demo in any visual form is a nice touch. The format can be a screenshot, a screen capture, terminal output pasted in a code block, or anything that captures the change well. GitHub supports both videos and GIFs in the PR descriptions.

In addition to a static demo video, it's a great practice to build preview versions of your application per PR branch. The ability to interactively test the application preview without setting up anything in a local development environment saves time and increases the likelihood of someone manually testing a pull request.

Remember that the fidelity of descriptions required depends on the context.

The spectrum of descriptions

You, as a team, need to figure out the perfect balance between explaining nothing or everything.

GitHub also supports issue and pull request templates if you want to standardize parts of the descriptions.

15. Use the shared repository model

For most private repositories, we recommend starting with the Shared repository model:

In the shared repository model, collaborators are granted push access to a single shared repository and topic branches are created when changes need to be made.

This model makes many aspects of the review process in GitHub simpler than the forking model that is popular among open-source projects.

16. Keep discussions public

It's convenient to have a quick chat about a pull request in the office, but be mindful of colleagues working remotely. It's polite to add a summary of face-to-face discussion as a PR comment.

Pull request discussions are searchable and easily accessible by all developers. They act as a history log of discussion which might be incredibly valuable when debugging a production incident later on.

Wrapping up

That concludes our complete guide for code reviews.

If you're interested in improving your code reviews beyond what GitHub can offer, take a look at what we've built in Swarmia. PR view enables convenient access to your open pull requests, Slack notifications help your teams review code faster, and working agreements help you follow the practices that you've agreed on together with your team.

Swarmia's pull request view showing open PRs as well as key metrics like cycle and review time

Additional reading

Top comments (0)