5 Biggest Mistakes Software Engineers Make In Their Code Review Process (And How To Fix Them)
Improve up to 50% your delivery without sacrificing quality by fixing these 5 mistakes that can make your code review process a huge bottleneck.
In my career, I have always worked using different types of pull requests plus code review processes.
I always considered code reviews as a tool to improve the quality of the code. CR is an opportunity to challenge your implementation, improve solutions, and enforce standards. CRs can help avoid bugs going to production, too.
Good teams use CRs to grow.
However, code reviews can quickly become a bottleneck.
Consequences Of Inefficient CR Process
Code reviews are one of the main bottlenecks I experienced in my career.
If you ever started working and found more than 20 comments on a PR, you know what I’m talking about. In my experience, I received more than 40 comments on a single PR. Teams changed, but at some point, we always ended up in the same situation.
PRs are blocked and waiting for reviews or comments to be addressed.
These are the consequences:
Context Switch: people need to continuously switch branches to address comments and interrupt their current work.
Time Waste: users don’t get value from your code until it is released to production. All those open PRs are NOT DONE. People spend considerable time reviewing code, addressing comments, and reviewing PRs again.
Conflicts: while PRs wait for review, their code is unavailable to the main branch. Long-living PRs frequently cause conflicts, mainly because people in the same team can work on the same parts of the codebase.
Dependencies: a standard solution to keep working while waiting for a CR is to create PRs starting from other PRs instead of the main branch. These practices might quickly lead to the need to continuously merge or rebase from the main PR to keep the depending ones up to date.
Harder Tracking Progress: Open PRs are not DONE but not IN PROGRESS. This means that a long PR queue makes your work harder to track.
There are multiple reasons why this happens.
The main reason why engineering teams generate inefficient CR processes is that they misuse CRs.
To be more precise, here are the 5 mistakes software engineers make in CRs.
1. Zero-Trust Process
Many people use PRs like they are used in open-source software.
Maintainers of OSS have a rigorous CR process because many contributions come from external contributors. For this reason, they must ensure that all the requirements are respected and the code satisfies all the standards. PRs might take a long time to be merged in OSS.
There is also the fact that many OS maintainers do that in their free time.
The same should not be applied to the company’s software.
When an employee creates a PR, the process should not be too strict. The reviewer should not pull the branch and test it locally. They should assume that the context of the PR is clear and ensure code standards are satisfied.
You and your team want to create the best possible code.
2. Async Discussions
I’ve seen many PRs blocked by these 3 types of comments:
Opinions
NIT
Misunderstandings
All these 3 types of comments should happen in a code review.
It is crucial to clarify that requesting changes in a pull request is a blocking action.
Blocking other people’s work for your opinion is a bad practice. Details can be fixed later on in another PR. Most importantly, misunderstandings should not be clarified asynchronously in comments. It takes too much time.
PRs are not the place for discussions.
3. Big PRs
When PRs contain many changes, everything becomes slower.
The review takes longer since the reviewer needs to go through more lines of code and files. This leads to PRs living longer and the necessity to update them and their depending branches. The result is, once again, more time spent on review.
But that’s not it.
Big PRs also lead to more conflicts.
When your changes involve more files, the probability of conflicts inevitably increases. Whenever there is a conflict, you must take time to fix it, and all this time is taken from actual project work. The bigger your PR is, the bigger the chance of having conflicts.
Long-living big PRs are the primary cause of Git conflicts.
4. Multiple Approvals Required
Many people think that having a review process involving multiple steps will increase the quality of the code.
That’s a lie.
The DORA report emphasizes that the quality improvement introduced in a process with many reviewers is irrelevant. Automated testing, clear ownership, and fast feedback have a significantly higher impact on the quality of the code. Introducing layers of CR in your process might significantly slow it and quickly become a bottleneck.
For this reason, it is not worth the cost.
5. External Approvals Required
Code ownership is crucial in code reviews.
When someone external to the team must approve a PR, it creates bottlenecks in the process. The team is the owner of the code. Team members should have all the context they need to understand the scope of a PR.
Reviews are significantly faster when team members are the only reviewers.
Whenever someone outside the team is required to be a reviewer adds friction.
This is necessary in some situations, like when changing a public API or an interface used by other teams. However, this should be the exception. When changes do not impact other teams’ code, reviewers should always be inside the team.
If managers, leads, or executives want to personally approve every PR, they are slowing down the process.
How To Fix CR Process
1. Require 1 Approval Only
Excluding specific scenarios, only 1 approval from team members should be required.
Each additional step will only add friction and slow down the entire delivery. People inside the team have more context, so they should be the only ones required to review PRs. All external approvals should be removed.
It doesn’t matter whether you have expertise or a role in the company; if you are not part of the team, you don’t have enough context to do a CR.
2. Automate
Automate everything you can.
All the steps that can be automated should be automatic without involving humans—tests, code coverage, lining, formatting, etc. Enable steps in your CI to verify that all these requirements are satisfied without wasting developers’ time. Receiving comments on a PR for lining is a waste.
Every step that can be automated should be done automatically before the review even starts.
3. Move Discussions To Sync Calls
Don’t leave comments to open discussions.
If, during a review, you find some topics that might open a more extended conversation, ask for a quick call. Discussing in a call will make the conversation quicker. An async conversation in comments might live for days, while a quick call can take minutes.
Moving a conversation from comments to a chat or a call can unblock a PR immediately.
4. Sync Code Reviews
People associate code reviews with comments in pull requests, but that’s not always true.
That’s what happens with async code reviews. Another method I used, and still use today, is called synchronous code reviews. In a sync CR, the author and the reviewer jump on a call to discuss PR changes. This method can speed things up since all the questions can be answered immediately.
I strongly recommend testing this method, especially if the PR is fresh.
5. Pair Programming
Code reviews are one method to improve code quality, but they are not the only one.
Pair programming is a practice where 2 developers work together on the same task. The CR can be skipped once the task is completed since the two authors reviewed the code. While many people think that 2 developers working on the same task is a waste of time, studies have proven that it is not.
Two developers pair programming effectively write only 40% less code than the same 2 developers working in parallel.
Pair programming introduces a considerable boost in the throughput of the entire process. Practicing it consistently moves the team to a status of continuous flow. There is no PR queue, and blockers are removed much faster than the async CR process.
If you want to start doing pair programming, you can check this post:
Did I miss something? How is your CR process? Let me know in the comments.
P.S.
If you like my work, consider inviting your friends to subscribe.