The way code reviews are commonly practiced is an example of cargo-cult programming, where rituals are performed with no understanding of how they work or what benefits are supposed to come from them.

Good code reviews should:

• Share knowledge across the team of new features, coding patterns, and critical bugs.
• Allow the team to discuss and agree on new coding standards.
• Support new developers in learning the team's coding standards.

Instead, the code reviews practiced by many teams:

• Only involve one reviewer.
• Focus on individual opinions and do not produce agreements on standards.
• Apply feedback to code after it has been written.

As such, these code reviews are providing little-to-no value.

Code Review Anti-Patterns

Blanket policy requires all changes to be reviewed

Requiring all changes to be reviewed means that there is going to be a high number of PRs, causing frequent interruptions for the developers who are being asked to perform the reviews. The number of interruptions only increases as the team becomes more productive or grows in size.

More frequent PRs turn them into a chore, and discourages developers from participating in reviews. The same one or two developers may well take on the work of reviewing all the PRs, feeling that it allows the rest of the team to "get on with work". (It is telling that reviewing PRs is not considered "real work").

Making PRs a chore, and allowing them to fall on one or two reviewers, leads to PRs being left open for an extended period of time, perhaps several days, until someone prompts the team to clear out the backlog. Those prompts become a repeating feature of the team chat. In the meantime, the team's workflow is impacted by not having all the latest code integrated into the main branch.

To try to reduce the frequency of PRs, developers will make each PR larger, often submitting only one PR when a feature is completed. The larger a PR is, and the more unrelated changes it contains, the harder it is to perform a meaningful review, but the team is actively discouraged from submitting smaller, more self-contained, changes.

Finally, a blanket policy for code reviews also means that many of these interruptions will be for changes that are trivial - a one line change, a CSS fix, a logic bug.

No agreement on what standards are being applied.

Rubber-stamping

Nit-picking

Inconsistent outcomes across reviewers.

Only one reviewer required.

Differing opinions

No shared lessons or agreements

No "whole team", team division

Recommendations

Code reviews and pull requests are optional, and all developers have the permissions to push directly to any branch. Partly, this is to support trunk-based development, but it is also because code reviews, as practiced on many projects, are a waste of time that generate little-to-no value.

Small changes do not benefit from being reviewed

Most changes should be low-impact

Reviews should be reserved for high-impact changes

Code review structure

What to look for as a reviewer

How to know if this is working

• All developers should understand what is expected of them in terms of patterns and coding standards. They should be trusted to work day-to-day without direct supervision or the need to have all changes approved.
• Developers should feel empowered to request a code review whenever they feel that they, or the team, would benefit.
• Reviewers are allowed sufficient time to familiarise themselves with the changes before the group call.
• Reviewers are not obligated to participate in every review, but there are enough reviewers to provide meaningful feedback and have the authority to make design decisions.
• The author understands what is expected of them in terms of leading the review.
• Reviews are not so frequent as to take significant development time away from the team.
• Reviews are frequent enough to catch high-impact changes.
• Code reviews are felt overall to be providing value to the team.

---

There must be no blanket code review policy that requires reviews on all changes. Such a policy leads to low-to-zero value reviews, rubber-stamping, and is not compatible with trunk-based development. An established team should have a good understanding of how code is expected to be written, and what patterns and standards are to be applied. New developers joining the team should have these expectations explained to them and given extra supervision as they get up to speed. Beyond that, developers should be trusted to get on with the work.

Small, trivial changes do not need to be reviewed. Examples would include one-line bug fixes, typo fixes, documentation, or applying the teams agreed coding conventions. Requiring code reviews in these cases is a waste of time - there is no meaningful review that could be given. Because they are low value, developers don't want to interrupt their work to approve them, so they wait for hours or days and become bottlenecks. Because the review process is a bottleneck, developers are discouraged from making these types of changes in the first place (missing an opportunity to improve code quality), or they get rolled into a larger, unrelated, review (making that review more difficult).

Most code changes will be larger, and non-trivial, but are still unlikely to be controversial. Once the team has been established on the project for a few weeks, most changes are going to involve the applications of patterns that have already been agreed on. To truly review a change like this in detail would require a significant amount of time. The reviewer would have to understand the requirements, read through all of the changes, possibly be shown a demo by the author, or run the code locally. At the end of all this, the review comments are likely to focus on small details - nit-picking, or matters of opinion. These code reviews again become bottlenecks, and developers become trained to quickly scan the changes and click Approve.

There are code changes that would genuinely benefit from being reviewed, either because they introduce new patterns, address bugs in existing patterns, or have widespread impacts (for example, a large refactoring). By this point though, developers have already become used to treating code reviews as a formality, and spent a significant amount away from writing code to do so. They are unlikely to identify these reviews as "special" and put in the extra effort they would benefit from.

Code reviews interrupt trunk-based development because they require that changes be made on a feature branch. The developer has to wait for those changes to be approved before they will appear in the trunk, and has to work in some sort of temporary branch in the meantime that they will then rebase. This friction will encourage developers to make fewer, larger, changes, which is the opposite of the desired outcome.

Conclusion: Code reviews should be optional. All developers have permission to push directly to the trunk and should be encouraged to do so in small, low-impact, increments that follow established patterns and standards. Developers should also be encouraged though to request a code review for anything that is larger, more impactful, or that they simply feel would benefit from more visibility.

Code reviews should not enforce a minimum number of reviewers. If there is value to be gained from a review, it only makes sense that the whole team is involved to maximize that value. The author should also have permission to approve their own code review - remember that we should trust developers to gather feedback and make changes; they should not require someone else's "approval". Even though the whole team may be giving feedback, an author should never feel bullied into making changes they don't agree with. Unless there is a strong consensus among the team, authors should generally be given latitude to write code the way they want to.

Code reviews should be resolved as soon as possible. Do not allow reviews to sit for hours or days awaiting approval. When the review is requested, give the team 15 minutes to look at the changes individually, then schedule a group call - it will be much faster to discuss any feedback verbally, rather than by passing written comments back and forth. If further changes are agreed upon, the author should be trusted to complete them without a further review.

Code reviews should have a structure that avoids any time being wasted by not knowing what to do, or low-value reviews from not being consistent. The author should lead the group call and share their screen. Summarize the requirements of the bug being fixed, and explain what prompted the request for a review. Give a demo of the change. Summarize the straightforward code changes, and emphasise anything controversial. Refer to the code review checklist and confirm that everything required has been covered. Then take feedback.