I am now going to present one of my more controversial views, my thoughts on code reviews. Yup, I do not believe all code should be code reviewed. What!? No code reviews. How can you ensure quality code? You will surely introduce many bugs. That’s disastrous! In my mind, I feel my view on code reviews should not be controversial, but talking with other engineers, other very bright and successful engineers, I know this is controversial.
Beginning where we should, what is the purpose of code reviews? To find bugs. To share knowledge about the code. To share coding style/practices. But primarily, it’s to find bugs. One of my biggest complaints about engineers is their blindness to not considering the business, specifically when it comes to ROI. Did you find a bug in the code review? Yes, thus it was worthwhile. Well, maybe not. If it took you 30 minutes to find a cosmetic bug, it’s not worthwhile. If it took 30 minutes to review code, you found no bugs, and you’re still confused about the code, it’s definitely not worthwhile. Taking ROI into account, code reviews need not happen under certain conditions
To answer the question under which conditions code reviews are not necessary, it’s easier to identify the conditions in which code reviews should occur. Mandatory code reviews should occur for new employees, for code in which the author is not the expert, and for any changes committed close to delivery date. When delivery is months away and you are the expert in the code, there is no need for a mandatory code review. The key word here is mandatory. If the code is particularly tricky and you want someone to review it, great, create a code review. Also, all changes to the code base should be communicated to the entire team. I prefer to have a hook so that a check in to git sends an email automatically to the team. The email will have the commit information and which files changed. Thus, the whole team, not just specific team members, have a chance to review the code on their own time.
The advantages of non-mandatory code reviews are many. It allows engineers to concentrate on a different task without having to go back-and-forth between tasks as code reviews can occur over days (do not underestimate the slowdown and inadvertent bugs created due to multitasking). The whole team, not just those invited for the review, participate in the process. Email hooks notify and record all changes. It places the change in trunk sooner so developers can integrate the change easily and immediately. It gets the change to QA sooner for testing, which is important for effective code base turnaround and bug resolution as discussed here.
The other main advantage of non-mandatory code reviews are obtained by avoiding the disadvantages associated with certain code reviews. Busy engineers can cause code reviews to be delayed for many days. When the author is the expert in the area, it becomes extremely unlikely that other reviewers will find meaningful bugs simply by reviewing code. It is also very likely that the reviewers will not fully understand the code. So you say, the reviewer should learn the code? No. You’re living in a fantasy world. I have been asked to be in code reviews where if I truly performed a valid code review it would take me hours and hours. Probably close to the whole day. All in the name of learning the code that I am not familiar with, probably will never touch, and will likely forget after 3 months. So, no, the ROI is not worthwhile, not even close. The bugs that are recorded in these types of reviews are almost always cosmetic and pedantic. I’ve found the best bugs are found by actually testing the code, not by looking at the code. Sure, certain bugs like race conditions and deadlocks are easiest found by inspecting the code, these often require thorough knowledge of the system to find, again, most suited for the expert in the area.
In sum, while all code reviews are *valuable*, not all code reviews are *worthwhile*. The benefits of completing the change, being able to task switch to something else, and allowing QA and other developers immediate access to the code, all weigh in on determining whether having a code review is worthwhile. In my experience, limiting mandatory code reviews to new employees, non-expert code changes, and when close to ship date, and performing non-mandatory code reviews on all other changes provides the most effective environment for software development.