Code reviews should be an essential practice in every software development team. As an industry, we are long past the discussions about whether to perform them at all, focusing on implementing them in the most effective way instead. As with many popular practices, some companies offer tooling that promises to lower or even eliminate the hard parts of it. You can tell from the title that I’m not a huge fan of those.
Problems with Code Review Systems
To be clear, I am writing here about systems that support offline (or ‘asynchronous’) code reviews. You send the code changes to another team member through the system and get their approval or suggestions back after some time. They are meant to save time by eliminating the need to meet and chase people, but they actually harm your development process in multiple ways.
Decreased knowledge sharing. Besides identifying code issues, another benefit of code reviews is an opportunity to share knowledge and experience between team members. And it works in both ways – the reviewer and the code author can give each other additional insight into a specific part of the system, technology, business domain, and more. It’s not something that would normally be entered in code review comments, many such things come up during a conversation between developers.
Communication issues. Teach your people to avoid talking to each other and you’re asking for big trouble. Giving ‘code advice’ is a very delicate matter, it’s easy to offend someone just by using the wrong tone of voice, and it’s even more difficult to control such effects in writing. Developers usually approach this in one of two ways: some avoid the possible confrontation by giving ‘standard’ comments, and others just directly write their opinion, not caring about how it will be interpreted. As you can guess, both ways cause problems.
Lower job satisfaction. Many developers actually enjoy showcasing their work when they feel it was done really well – it’s like showing our precious baby to friends. Having it reviewed by someone offline is not really the same thing. You don’t hear the reaction or see the look on their faces while you guide them through your brilliant solution…
Less pressure for quality. I believe that most people (not just developers) do a better job when they actually have to show and explain the results to someone else. We all want to appear smart (or at least avoid humiliation), and we are often willing to give extra effort for that. Again, it’s a stronger effect when you do that in person.
Interruptions. Do you think you’re limiting the interruptions in your team by queuing the code reviews in a tool? Think again. What you are actually doing is replacing a small interruption with a larger one. In most cases, it’s just more difficult to shift your mind from one coding task to another, than from coding to reviewing (and back).
For instance, imagine a developer finishing a coding assignment. He puts the task into Pending Review status in the system and starts development of another feature, shifting his focus to a different problem. Then, after a few hours (or even days), he receives a list of required fixes from the reviewer. He goes back to the previous feature, possibly interrupting the new task. How much time will he lose to switch back to the original context? How many little details has he already forgotten about? How many issues it will cause? What is his motivation to change a feature he has already (mentally) closed?
The Good Kind of Tools
Having stated the above, I definitely support and encourage utilization of static code analysis tools, like FxCop and StyleCop for .NET developers. I believe such tools should be an important step in every mature build process. They give you multiple benefits without any significant drawbacks, and require only constant (i.e. independent of project size) time to set up.
Another scenario is audit documentation, e.g. PCI. It’s easier to keep code review evidence for such purpose in a system rather than on separate forms, but it should only be used as a means to document code reviews that were performed, rather than as a replacement for them.
For every automation opportunity you need to carefully review both sides of the picture. While ‘offline’ code review systems can save you some time in the short term, the long term results will vary from some missed opportunities at best, to significant communication and quality issues at worst. I’m not saying ‘don’t use them at all’, but if you decide to utilize such platform then make sure it’s not used for all reviews, and it’s not an excuse to avoid direct communication and important discussions within the team.
At this point, a valid question would be: if we avoid using code review systems, how can we make our review sessions effective? That’s going to be the topic of my next article.