Why You Should Not Enforce Code Reviews

Code reviews are a vital part of a good software development lifecycle. Code reviews are great for knowledge sharing, ensuring team members are familiar with recent code changes, getting feedback on your work, maintaining high coding standards and occasionally it may also help catch a bug or two.

Today, many companies take pride in their solid development processes and having a policy where every code change gets reviewed. In fact, it has become a common practice to force a code review before being able to merge a Pull Request (PR), while having the option to commit directly to the main branch blocked.

It may sound counterintuitive, but at Duda, we believe that forcing a code review for EVERY change, does more harm than good, and believe that optional code reviews actually result in higher quality reviews, greater ownership, better developer experience and increased productivity. Here’s why…

Code reviews aren’t meant to catch bugs

One common argument is that finding bugs in production is 5X-10X more expensive than catching them during dev time, and because even the smallest change may cause a bug, reviewing even the one liners is worthwhile.

While it’s great if a reviewer manages to spot a bug during a code review, code reviews are not meant to serve this purpose and shouldn’t be seen as a reliable, systematic approach for preventing bugs.
Other than the fact that reviewers can’t possibly run all the different execution paths in their head, the purpose of code reviews, as listed above, is mainly for knowledge sharing, getting feedback and creating alignment on coding standards and best practices.

Many studies have been conducted on this matter, and while all agrees code reviews are incredibly valuable, they fail to agree its effectiveness at finding bugs. For example, Microsoft’s research showed only 15% of code review comments helped finding bugs, let alone major blockers.

There are more effective tools for spotting bugs or security threats even earlier, like doing design reviews before coding, adding a static code analysis and penetration test tool or adopting a TDD coding methodology.

Trust

Do you trust the developers in your company? I hope you do, because the modern day developer does way more than just coding, developers do their own technical design and breakdown, perform research, write unit tests, review other members’ code, deploy to production, monitor their changes and handle production incidents. These tasks require a working environment with a high level of trust.

If that’s also your case, why would you tell a developer “I want every little thing you do to get a second look.”?

Does this encourage a trusting environment? What kind of message does it send to that developer?

Let your developers decide for themselves if their PR would benefit from a review or not, you probably trust them with more important decisions already. And by doing so, you’re showing once again that you trust their judgement and are taking the trusting environment a step further.

It will slow you down (sometimes, for no good reason)

Let’s say you’re a frontend developer and you just merged a PR of a shiny new feature. Since it’s a significant change, you asked two team members to review your PR. After some comments and fixes it was accepted and merged. Great Success!

Now, one of the UI designers approaches and says, “Sorry, I missed something. That title is too big. It’s H2, but should be H3, can you change it, please?”

“No problem”, you say! It’s just changing that `headingType` prop from H2 to H3 - exactly 4 seconds of work.

Does this really need to get reviewed? What value does it add to the dev process, other than slowing it down? This just adds unnecessary friction to the process, reduces productivity (and happiness) for the developer and adds yet another context switch for the reviewer that probably has nothing to say other than the infamous “lgtm” (looks good to me).

The reviewer angle

Ping! Slack bot alerts one of the developers that a PR is assigned to them, this must get reviewed otherwise it can’t get merged, right? So sure, devs will review all the PRs and will try to get it done quickly, so as not to block the process, BUT .. will they review it as thoroughly as if they knew the author of the PR explicitly asked for their review because they believe they can contribute something intelligent to it?

I would argue that if the code review is optional, the reviewer will feel accountable to go even deeper and provide a higher quality review, in contrast to if it was “because of policy” or, if the author “just needs this quickly reviewed, please”.

When everything gets reviews, all the time, reviewers might struggle spotting those risky PRs that really need careful attention, as everything is part of the routine.

Accountability

At Duda we see developers as complete owners of the end to end delivery of their tasks. Developers aren’t just responsible for the pure technical aspects like writing quality code, but are accountable for making sure features go live and are rolled out safely to our customers.

Developers may ask a QA team member for assistance if they want a second look, but it’s their decision. Code reviews should follow the same principle. Devs make the choice whether to ask for a review or not, and are held accountable for their choices. If a developer did not ask for a review and still broke something, they will do a retrospective with their team, reflect on their decision and (hopefully) learn from it.

Our philosophy prefers moving fast and encouraging taking some risks, rather than adding more processes and trying to avoid the inevitable, occasional bug. We prefer to invest in logging, alerts and monitoring tools to discover issues as quickly as possible, along with tools to reduce bug impact like a Feature Flags management system, gradual feature rollouts, deploy-on-demand CI/CD workflow and automatic rollbacks for quick recoveries.

We also heavily rely on short living branches (trunk based development) and deploying small, incremental changes to reduce risks and shorten root cause analysis when bad things do happen.

Obviously all of these aren’t meant to replace code reviews, but to increase developers’ confidence and reduce the risks of pushing code to production. Usually it’s the other way around, when those tools don’t exist, companies tend to turn to code reviews to replace them in order to provide some (false) sense of confidence in the dev process.

Code reviews are still incredibly important

Having said all that, we’re still BIG believers in code reviews. We see code reviews as a fundamental part of our day to day work and a crucial part of our ability to successfully onboard new members, maintain code quality, ensure good code readability and share knowledge.

Since we acknowledge the importance of code reviews but don’t force doing them, we measure it and strive to get better.

Today, most teams have around 70% of their PRs reviewed, but since we want to do better, we set a quarterly KPI to see this number increase. However, 100% will never be our goal.

In order to meet this KPI and keep improving without adding any strict policies or rules, we constantly advocate the importance of code reviews and the major advantages of doing them, and we also recognise individuals who spend a lot of their time reviewing their peers’ work.

While this approach has worked really well for us, it might not work well for others, and that’s totally fine! In fact, there are probably use cases where it makes perfect sense, open source projects are the obvious example, but this also applies to shared common libraries widely used by a lot of teams in a large organization.

The goal of this post is to encourage having a conversation about whether or not a strict policy will do your teams good and to give an alternative opinion to the common paradigm of “always do code reviews” and demand it by hitting that checkbox in your VCS.