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.
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.
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.
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:
DRAFTYou can open a PR in a draft state to show progress or request early feedback.
READY FOR REVIEWYou 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.
CHANGES REQUESTEDFix 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.
APPROVEDThe author is responsible for merging their own PR.
Yours might look different, but as long as the team agrees on the process, all is good.
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".
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.
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.
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.
Smaller batches are easier to design, test, review, and merge. There are studies   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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
That concludes our complete guide for code reviews—and you know what that means in a company blog: a quick sales pitch!
If you're interested in improving your code reviews beyond what GitHub can offer, take a look at our code insights. Swarmia’s pull request 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.
- Google's Engineering Practices
- How to Make Good Code Reviews Better by Gergely Orosz
- How to Make Your Code Reviewer Fall in Love with You by Michael Lynch
- Code Review Guidelines for Humans by Philipp Hauer
- Reducing Pull Request Cycle Time with Swarmia