Code reviews (https://about.gitlab.com/topics/version-control/what-is-code-review/) are a fairly standard practice in the industry, especially within larger companies. The process of having multiple developers look at code changes critically is found in several development methodologies (eg: extreme programming, paired programming, etc.), and they are often perceived as essentially for maintaining a level of overall code quality. I'd imagine that no respectable engineering leader in any larger organization would accept a process which did not incorporate mandatory code reviews in some form.
So with that intro, here's a bit of a hot/controversial take: I think code reviews are overrated.
Before I dive into why I think this, a quick tangent for an admission: the majority of code I've actually written in my career, personally and professionally, has been done without a formal code review process in place (and thus, not code reviewed). I've also, personally, experienced considerably more contention and strife generated by code reviews than value-add. So I certainly have a perspective here which is biased by my own experience... but I also don't think my experience is that unique, and/or would be that dissimilar to the experience of many other people, based on conversations I've had.
So that being said, why do I not subscribe to the common wisdom here?
I know a manager, who shall remain unnamed here, for whom the solution for every problem (bug, defect, customer issue, design oversight, etc.) is always one of two things: we should have caught that with more code review, or we should have caught that with more unit testing (or both). His take represents the sort of borderline brainless naivety which is all too common among nominally technical managers who have never accomplished anything of significance in their careers, and have managed to leverage their incompetence into high-paying positions where they parrot conventional wisdom and congratulate themselves, while contributing no positive value whatsoever to their employing organizations.
The common perception of this type of manager (and I have known several broadly with this mindset) is that any potential product failure can be solved by more process, and/or more strict adherence to a process. There is not a problem they have ever encountered, in my estimation, for which their would-be answer is not either adding more process, or blaming the issue on the failure of subordinates to follow the process well enough. To these people, if developers just stare at code long enough, it will have no bugs whatsoever, and if code which passes a code review has a bug, it was because the reviewers didn't do a good enough job, and should not have approved it.
Aside: The above might sound absurd to anyone who has spent any time working in the real world, but I assure you that it is not. I know more than one manager who's position is that no code should ever be approved in a code review, and/or be merged into the company's repository, which has any bugs whatsoever, and if there are any bugs in the code, the code review approver(s) should be held accountable. I think this is borderline malicious incompetence, but some of these people have failed upward into positions where they have significant power within organizations, and this is absolutely a very real thing.
Back to code reviews, though: I think they are overrated based on a couple perceptions:
- The most important factor in producing high-value code over time is velocity (of development), hands down
- Code reviews rarely catch structural design issues (and even when they do, by that time, it's often effectively too late to fix them)
- Code reviews encourage internal strife from opinionated feedback, which often has little value on overall code quality
- Code reviews often heavily implicitly bias against people without as many social connections, and/or those which do not "play politics" within the org/teams (and conversely, favor those that do, encouraging that behavior)
- As per above, code reviews are very often abused by bad managers, which can easily lead to worse overall outcomes for orgs
To be clear and fair, code reviews have some tangible benefits, and I wouldn't necessarily dispose of them all together, were I running a dev org. In particular, the potential benefits such as sharing domain knowledge, increasing collaboration, and propagating best-practices (particularly when mentoring more junior developers) are tangible benefits of code reviews or equivalent. There is a reasonably compelling argument that, with good management in place, and when not used for gating and/or misused for blame attribution, code reviews have sufficient positive value to be a good practice.
However, the risks here are real and substantial, and this is not something which is a clear win in all, or perhaps even most, cases. Code reviews impact velocity, and the positive value proposition must be reasonably high for them to have a net positive value, given that. You're not likely to catch many actual bugs in code reviews, and if your developers use them as a crutch for this (psychologically or otherwise), that's another risk. If you have management which thinks thorough enough code reviews will give you "pristine" code, you're almost certainly better off eliminating them entirely (in concept), in my estimation (assuming you cannot replace those terrible managers). Code reviews are something which can have net positive value when used appropriately... but using them appropriately is something I've personally seen far less often than not.
That's my 2c on the topic, anyway.
No comments:
Post a Comment