Code reviews exist for three reasons.

1. To make sure the changes are safe to merge
2. To improve the overall health of the code base over time
3. To share knowledge across teammates

To move fast over the long run none you can't skip out on any of these.

If your team is merging changes that aren't safe (they contain bugs, security issues, aren't doing the right things, etc) you'll be immediately needing to fix the code - slowing you down.

If your team is merging changes that reduce the codebase health you might feel like you're able to move a bit quicker by getting those changes merged in. But long term you'll wind up making the code harder to work with and slow things down.

If your team is merging changes with no one else on the team seeing them, then new solutions, ideas and best practices are not shared, code gets duplicated, and people become a single point of failure.

Each of these aspects of code review help you move faster over the long run, but the way you check for each of them during reviews impacts how fast you can move as a team.

The Inefficiency of Current Code Reviews

While code reviews can be very important to prevent future slowdowns for a team, the actual code review process today is extremely inefficient.

The average code review takes more than 21 hours and roughly 2/3 code reviews don't result in any code changes, suggestions, or feedback on the PR. And developers on average spend 8.6 hours every week reviewing their colleagues code.

That means developers are spending ~5 hours a week on average reviewing code changes that they don't actually need or get useful feedback. And those code changes need to wait for an extra day before getting merged.

On top of this that time delay between opening a pull request and getting review feedback means that you've usually moved onto something else by the time you do actually get feedback on a PR - breaking your flow and slowing you down further.

All this adds up to a current review process that is slowing things down in the short term - even if it has long term benefits.

With Sourcery code reviews we set out to try to design a review process that would help speed things up both in the short and long run - starting by how we approach approving PRs.

Blocking Only What Needs to Be Blocked - Approving By Default

At a company the vast majority of code changes are going to be safe to merge. They aren't going to break the code, they aren't introducing major risks, and they're doing what they are supposed to do.

These types of "safe" code changes shouldn't need to wait to be merged in - you should aim to be moving as fast as possible with them. This reflects my view on how to approach code review decisions in general - you should default to approving every change and only not approve those changes that would actively harm the code base.

As we built out the decision making engine within Sourcery Code Reviews we wanted to introduce this ideal of approving by default to help teams move as fast as possible.

To do this we had to figure out what should and shouldn’t be a blocking issue. For us a blocking issue is:

• Any potential security risk
• A bug that would cause the code to break
• A PR that isn’t achieving the job it was supposed to

Some things that aren’t blocking issues:

• Minor code quality issues & refactoring opportunities
• Content improvements or typos (unless they would somehow result in breaking bugs)
• Code complexity issues
• Open questions about the changes

Most Improvements Shouldn't Be a Blocker. But That Doesn't Mean We Shouldn't Make Improvements

Even though minor issues shouldn't block a merge, we still should look for them during code reviews. Identifying these types of issues is still a critical piece of the code review process and you should aim to be exhaustive in every review. But, by decoupling the discussions around how to improve the code from the approval/rejection decision of a PR, the author of the PR can make an informed decision about which of these improvements to immediately make, which can be left for future work, and which require further discussion without needing to wait for re-approval.

Note - this still has a fair bit of nuance to it. If there is a code change that is introducing so much complexity to the codebase that it would be an immediate hindrance then this should still be a blocking change. But these tend to be few and far between

Knowledge Sharing != Reviewing - Decoupling Reviews and Knowledge Sharing

While we often use code reviews to share knowledge, there's no need to tie this learning to the moment a change is ready to merge. As we move more and more towards a world of automated code reviews, the knowledge sharing process will become more decoupled from the review itself. Ideally you can have knowledge sharing opportunities within your team during the planning and early design phase and then can allow for asynchronous knowledge sharing of what changes were made to the code base after a change was merged in. There's no reason that a change should be slowed down from being merged in for knowledge sharing during reviews.

Speeding Up Code Reviews

Code reviews are critically important to make sure the right code is being created in the right way and to allow a team to move as quickly as possible in the long term. But that shouldn't mean you are allowing reviews to slow down the development process. At Sourcery we are building an automated code reviewer to give you more in depth feedback around your code changes faster.

If you want to try out Sourcery on your projects you can set it up for any GitHub repo or if you’re using another code hosting platform let us know.