Why Code Reviews Are Not The End All, Be All

Oh, the joys of code reviews. You learn a lot, and also get to teach other people. Such a simple procedure, which helps catch bugs before they are merged, while spreading experience organically throughout the team. What's not to like?

Well I tell you what's not to like.

10 lines of code = 10 issues.

500 lines of code = "looks fine".

Code reviews.

Too trivial or too critical

Let's face it: Some code changes do not need review. Having someone else review a refactor has no intended functional changes is a waste of effort. The author already double-checked the (mostly mechanical) code changes a dozen times, so the reviewer, which is eager to move on to other stuff -- and rightly so--, is just as likely to miss an introduced bug.

In other cases, some code changes are a one-liner which, if deployed in the next two hours, would dearly help getting that fix into production before more complaints trickle in. Mandatory Code Reviews just make this process longer and more stressful.

When they are valuable

Don't get me wrong. Code reviews are dearly valuable for junior members of the team, for those unfamiliar with the repository, architecture or the patterns being used. However, in those cases, I tend to place trust that author themselves will realize they are out of their depth, and will seek for assistance and advice, tying the bow with a code review. I'm all in favor of that.

When they aren't

What really doesn't help anyone is nitpicking and blocking a Pull Request over expressing the same piece of code in a slightly different manner. If it is short, clear and easy to read and understand, it doesn't really matter how you say it. I'd even add that, the dumber and less fancy, the better.

I'd also like to point out that reusing some small piece of code, just for the sake of reusing code which is otherwise unrelated, is pointless and actually harmful. The phrase “A little copying is better than a little dependency” comes to mind. Code can always be simplified and abstracted if needed, but decoupling later is never easy. Never.

The real heart of the issue

Finally, the really valuable inputs won't ever come up in a Code Review. What really adds the most value is the proper design an the architecture of the system. These are the decisions that become blessings or pain points for months and years down the line.

The dire reality is that the reviewer will never have the time or inclination to fully understand the implications of the proposed change. The reviewer is perpetually tunneled into the lines that were changed, not the overall vision, so their advice is confined to that.

What then?

With all that in mind, my proposal is to:

1) Don't make code reviews mandatory. A senior enough team member can judge for themselves when its acceptable to not seek a review for a change which is either too small or too trivial to warrant additional attention, or too critical to wait for someone else.

2) A reviewer should approve the PR if there's no obvious or glaring blocking issues. Blocking a PR over trivial code changes -- which does not change the functional result -- is pointless. The author can figure it out based on your comments and reach you if needed.

3) System Design or System Architecture Reviews are infinitely more useful for adding value over time. These are the decisions which have consequences that span over months and years, fundamentally affecting developer productivity, system reliability and future maintainability.