How About Better Code Reviews
It’s probably fair to say that anyone with experience in software engineering will also have at least some experience with code reviews. After all, it is an established practice in the field, with the first formal code review process (Fagan inspection) being described in 1979. They are also quite popular. Depending on the study, it is estimated that around 90% of software development teams adopt some form of change-based code review. This suggests that they are considered a good standard practice. However, that’s not always the case.
In fact, code reviews are a common subject of discussion and publication, with an increasing number of people saying that they are harmful. Some even say that we should get rid of them. These opinions are not without merit, as many code reviews tend to be bad.
Why Code Reviews Tend To Be Bad?
The easy answer to “Why code reviews tend to be bad?” would be “Because of human nature”. When you read that, it's likely that your mind quickly jumped to a memory of a situation in which you received a lot of comments on your review, half of which were nitpicks and the other half of which described edge cases that “should have been obvious” to you. Or maybe it brought back that frustrating feeling of power imbalance when you were a younger member of a team and your pull requests always had to go through multiple iterations of code review, while the most senior members got theirs rubber-stamped without a question? Yes, these situations are bad and silently destroy the team's effectiveness and morale. But these are just symptoms. The aspect of human nature I have in mind is the false sense of understanding that causes leaders to make code reviews look like this.
Even leaders with little technical knowledge think that they understand code reviews. After all, it’s a quality assurance activity in which people examine the source code. That seems self-explanatory. As a result, they perceive them as an easy-to-set-up quality gate - a checkpoint where senior developers will prevent bad code from reaching production. And they will put pressure on this. How many times have you heard a manager ask “Why didn't you find this during code review?”? I’m pretty sure that a lot more than “How can we adjust our automated test strategy so it catches issues like this?”. Focusing on code reviews as the most important quality gate makes them a bottleneck. Review cycles become longer and longer in the hope of “getting everything right”. Seniors become overloaded and trapped in constant context switching, which often causes them to become less thoughtful when doing code reviews. The only way to escape this downward spiral is to take as much of this quality gate responsibility away from code reviews as possible.
Move Most of Quality Gates Out of Code Reviews
For code reviews to be effective and valuable, as much as possible should be offloaded to automated quality gates:
Style and formatting rules should be checked by linters.
Quality should be evaluated using static code analysis tools.
Early vulnerability detection should be the responsibility of static application security testing.
Architectural rules should be enforced by architectural design pattern tests.
Non-functional requirements and/or architectural goals should be guarded by fitness functions.
Interactions with external components and services should be ensured using contract tests.
Functional requirements should be verified using unit, integration, and end-to-end tests.
Offloading all of the above (or more, or less, depending on the unique needs of each solution) enables quick feedback on the general quality of the code, allowing the conversation during the code review to focus on learning and knowledge transfer.
Make Code Reviews About Learning and Knowledge Transfer
Once the majority of quality gates have been automated, the purpose of code reviews will no longer be to prevent bad code from reaching production. This has already been achieved. Now, it's about sharing and discussing ways to improve, with the whole team participating in collaborative learning. The typical subjects of such conversations include:
Future maintainability.
Long-term performance, scalability, and reliability considerations.
Alternative approaches and trade-offs.
Introduction of new best practices and patterns.
Nuances of domain knowledge.
The nature of the conversations will also be different. Rather than comments such as “This is wrong” or “Fix this”, typical conversation starters will be statements such as “I can see two approaches here… and the trade-offs are…”, “The current code works, but I would also like to explore this pattern as an alternative…”, or “This is an interesting approach. Can you elaborate on why you chose it?”. These types of comments promote knowledge sharing, exploration of alternatives, and investment in future learning. And these comments don't only come from seniors 🙂.
Last but not least, those conversations have a natural optionality. If there is no time to discuss them right now for any reason, the discussion can happen later because the quality criteria have been met.
Monitor for the Outcomes You Desire
The typical metrics around the code review process don't reveal much about the qualities I have described here. While you will see an impact on “time to merge” and “number of review iterations” when you automate the quality gates, the learning and knowledge-sharing aspect is more elusive. The first visible signal should be the team's attitude towards code reviews: are they looking forward to them, or dreading them? Another useful piece of information is how confident different team members feel when working with different areas of the codebase. If confidence levels are low, you may need to work on the review rotation strategy. It is also worth monitoring how quickly new patterns spread through the team and whether all members are asking increasingly sophisticated questions, as this will allow you to assess whether the team is growing. This is important, because good code reviews should be a growth engine for the team.