Manual Code Review of every single change is OVERRATED

Many software companies have this policy that every change has to be manually reviewed and approved by some peer (or senior) developer either before it is commited or before it is merged in main development branch. This practice is recommended by many software luminaries. However, when implemented in a project/product, I found many practical issues with 100% manual code reviews and reached a conclusion that "manual code review of every single change is OVERRATED"

Over years, I realized that manual code review of every single change is OVERRATED. It does not appreciably reduce post shipment defects but does introduce many inefficiencies in your development process. Manual Code Review of every single change usually have -ve ROI. I find that nobody writes about bad side effects of forcing developers into 100% manual code reviews. Recently I found one article and that triggered me to write this blog post.

So here is the list of problems that I found in 100% Manual Code Review process

Problems in Manual Code Review of every single change

1.  ROI on Code Review - 
   
   Is reviewer finding kind bugs in code review such that if they remain undetected will take more efforts than the time spent in Code Review ? If yes, then ROI of Code Review is +ve else ROI of Code Review is -ve.  Most of the Code Reviews have -ve ROI.  
   Reviewer may be a good developer but he may not be really good or may not have an 'eye' for defect. Some years back I did an informal study of 'code review' comments reported in my company's code review system. Majority of the code comments were about 'coding style' or 'comments' or 'naming conventions'. Very rarely a serious bug is detected. Effectively Such code reviews have a '-ve ROI'.
2. Reviewers time and attention.
   
   Typically Reviewer is mostly one of the senior developers. He/She also has their own commitments on new feature development. Usually these commitments take priority over code reviews. Hence Code Reviews remain stuck in queue and total feature development time increases unnecessarily (elapsed time from start to feature is delivered in end user's hand).
3. Team's Most productive developers are 'reviewing' rather than 'writing new code'.
   On the other hand, if Senior developers spend majority of their time doing reviews, then 'most productive developers' are effectively not writing any code. Either case results in lower productivity for the entire team.
4. Lower developer productivity while fixing bugs reported during code reviews.
   Code review is helps only if review happens within short time writing the code (preferably within a day). However, in general actual review happens after many days. In this time, developer loses the context of code. When finally the code is reviewed and he/she has to fix something in the code, it takes longer because now developer has to 'get in the context' again. Effectively it lowers the developer's productivity again.

Over year's almost every team that I have worked with has made this mistake and effectively reduced the productivity. How do we fix this ? Whats the alternative ?

How to regain the productivity lost by "manually review every code change policy" ?

1. Institute "Zero Warnings" policy for your code.
   This is easiest and most effective way to eliminate bugs. Every new version of compiler improves in ability to detect potential problems and report them as 'warnings'.  Set your compiler to 'highest warning' level. If you are using Visual Studio, turn on 'report warnings as errors" setting. Fixing warnings is 'practically no-brainer'. Warnings have to fixed immediately. DO NOT wait till end of sprint or end of release to fix warnings.
2. Take advantage of Static Analysis Tools for your programming language.
   Every programming language now has static analysis tools (linters, tools like Klocwork, Coverity etc). This Wikipedia article will give you a list of static analysis tools. Make static analysis part of your 'CI/CD' pipeline. Make sure that static analysis bugs are fixed within day. DO NOT wait till end of sprint or end of release for fixing static analysis errors.
3. Identify your critical files and review only those manually.Stop reviewing very single line of change. Identify your critical files (use complexity analysis, analyze version history to find most frequently changed files etc).  Usually 80-20 rule applies 20% are your critical files. Mark those files in system. Any commits in these files should trigger your manual code review process. Manually review these critical files ONLY after warnings and static analysis bugs are fixed.

Development processes/practices followed in a typically software development team usually have serious inefficiencies. By using 'common sense' (rather than following common practices) it is possible to achieve an 'order of magnitude' (2x to 10x) improvement in the team's productivity and delivery quality. It DOES NOT require a change in methodology (like move to Agile) or any new expensive tools. It requires some disciple and taking advantage of many low hanging fruits. The results are usually visible in about 3 months. 

[Shameless Plug]If you want to know how to improve your team's productivity by 2x to 10x, I am available for 'consulting/coaching'.

References

1. Code Reviews Do Not Find Bugs - How the Current Code Review Best Practice Slows Us Down
2.  

My code review checklist

I am not a fan of checklists (especially for code reviews). Code review checklists start small and then slowly become really large and unwieldy. After sometime checklist becomes a bottleneck and instead of improving effectiveness of your process, these lengthy checklists start reducing the effectiveness. 

However, there are situations where I used checklists and they worked very well. For example, a Customer Release checklist. There are many small small things that you need to do before sending the new release to customer. Its easy to miss few critical steps. A release checklist has always worked very well.

I was not sure why in typical organization sometimes checklists did not work well (for example, in cases like code review) while sometimes it really worked. What exactly is the difference ?

Sometime back I read Atul Gawande's book 'Checklist Manifesto'. It triggered my interest in Checklists again. As first step I extracted a Code Review checklist from my code review training content. I have used this 'mental' checklist for a many-many years. It has worked well for me even with different programming languages (C/C++, Java, Python, C#, Javascript) and technologies. 

Here is my code review checklist.

Code Review Checklist from Nitin Bhide

PS :  Based on my experiences, information from Atul Gawande's book and from information internet, I have now prepared a 4 hour hands-on session on creating and improving the checklists. Contact me if you are interested.