G to the Square
Should we always code review?2019-07-10
I recently "stumbled" into the following statement from the Technical due diligence calculator for startups on Hacker news:
7.c Do you have code reviews? (+5p) Only critical routines (-1p) YES. 100% coverage (-5p) NO (0p) Other Our opinion: At the early stage, it's important to keep the right balance between speed of processes and reliability/robustness. Please, do review and unit test at least critical paths.
Wait, what!? I thought (in theory) that all code should be peer reviewed? There are clear benefits:
- Knowledge transfer
- Assist on boarding of new employees
- Helps catch errors
- Helps assert the solution as proper and meets the requirements
- Validates that the code is readable and understood by another person
- Helps verify the coding style guidelines are followed
- Raises overall quality of code because we know peers will read our code.
Still they come with side effects:
- Badly written requests explaining what the code does can lead to people blindly accepting it because they "trust" the developer.
- They can become time stealers when they are too big and the reviewer needs to spend a whole afternoon or day understanding it
- Can lead to too much context switching
- Can block continuous integration when there is nobody available to review it.
- Gatekeepers may arise. These people will want to assert they some sort of superiority or own the codebase, by rejecting the code due to nuances that have little to do with the actual code.
- Finally there is the issue that most people are bad at giving constructive feedback
Code reviews are more complex than we think. Feelings can be hurt, conflicts arise and add an overhead to software development process. So, the question is are they needed? If so, I agree that 100% of the time is not healthy in case a company or team want to have fast deliveries and keep the morale high. This last point we don't think about it much but is important because something that makers love is seeing their hard work being fruitful.
So, are they always needed? No.
And when should they be performed?
Based on the explanation of the statement that triggered this post, it should be done only for "critical routines".
Then, what makes a critical routine?
If there is a risk of the company losing money in case the code fails, then it is critical. Otherwise, it will be ok to merge without a code review.