3 ms·
I agree with everything in the article. My biggest problems with code review are: 1. The reviewer usually lacks the context to truly review the solution and a
by caymanjim 8y ago
I agree with everything in the article.
My biggest problems with code review are:
1. The reviewer usually lacks the context to truly review the solution and all its implications, and instead gives only a cursory review focused on style and trivialities. Sometimes they're familiar enough with the code to give an insightful review, and there's benefit to commenting on even the small things, but I've seen countless major architectural problems pass review because the reviewer lacked the time and familiarity to notice them. This isn't helped by reviews typically being done on GitHub code diffs; you're seeing a couple out of context lines and it's hard to understand how it all fits together.
2. By the time code is ready for review, it's too late. Sure, you can stop the presses if you notice an egregious problem (not that anyone usually does; see #1). My team tries to keep its reviews small and frequent, but they still often contain one or more full days of effort. If the solution has major problems, catching them after the fact means a lot of wasted effort on rewrites. Often there's no time to fix it right, so someone hacks around the faults and ships a "good enough" product. The tech debt piles up quickly.
As a result of both of these, code reviews are often little more than a rubber stamp.
This isn't to say that I don't think code reviews have any merit. I think that code reviews on every single change are a waste of time. Periodic detailed reviews of architecture are valuable, and periodic team reviews of code can help spread good patterns.
Many of these problems are obviated by pair programming. It's real-time code review before it's too late. It's the single biggest benefit to pairing, although there are plenty of others (like sharing knowledge and building team bonds). Pairs are also subject to the same pitfalls, but at a far lower rate than solo developers.
- v_lisivka 8y ago1. You need to put context into comments. Code is written once but read multiple times, so you should optimize for later case. 2. "Too late" is when bug hits the client and affects income. If not, then it's not "too late", it's just "a bit late". 2-3% in effort increase to decrease number of bugs (increase customer satisfaction) by 15-20% is huge win. If you will not do that, your competitors will do that instead of you.