Code Review and How Enterprises Can Miss The Point
Editorial Note: I originally wrote this post for the SmartBear blog. You can check out the original here, at their site. Check out my posts and some of the others and take a look at their products as well while you’re there.
If you work for a smallish company, as part of a modestly sized software development group, the path to a code review policy is likely a short, direct one. It could be as simple as a respected team member or the manager saying, “hey, let’s start doing code review.” But whatever the impetus, the participants will tie the process closely enough to the desired outcomes from it to adapt it as needed.
The Capital E Enterprise
At the enterprise level, the calculus changes considerably. And when I say the enterprise, I mean The Enterprise – a size and scope mammoth enough to demand capitalization, even when the rules of grammar do not. These are companies so big that those who work at and have worked at them will assure you that there is simply nothing out there that’s comparable. There are, they will tell you, an entirely different set of rules that apply. And they’re more or less right.
The Enterprise loves structure and hierarchy, usually of the command and control variety. The scale is so immense that the only structure up to the task is a pyramid reminiscent of a military chain of command. The organization lumbers along like a battleship, powerful, majestic, and requiring incredible teamwork, cooperation, and precision to change direction in any way.
In this environment, code review tends to make its way to development team in a very different way and for very different reasons. Of course, even in a massively homogenized environment, one size does not fit all for the development teams. But the cog in a larger machine dynamic makes the following sort of scenario much more likely.
A Study of Motivations: CTO
In a large conference room with impressive windows, a formidable view, and glass pitchers, the CTO or sits, looking at a power point presentation about budget. She sees that a jaw-dropping 60% of her spend on software developer labor is dedicated to break/fix efforts with their own software, leaving a woefully inadequate slice of the pie for new feature initiatives.
Chagrined, she solicits ideas from her lieutenants in the room on how to move the needle down from the 60% mark. After some other suggestions, a promising idea is floated with some actual industry numbers to back it up: “peer code review has shown in at least some studies to be the most effective mechanism for eliminating defects.”
And so was born The Great 2016 Code Review Initiative. A committee is convened, messaging is drafted, a slide deck is smart-arted, and every single software developer is shown a 5-phase rollout plan for The Enterprise’s Great 2016 Code Review Initiative. By Q4, 2016, every new line of code, in every one of the organization’s thousands of code bases, will be reviewed by one other person.
A Study of Motivations: Middle Manager
In a medium sized conference room, with a modest view and a Keurig, a middle manager looks around at his direct reports, the gleam of opportunity flashing in his eyes. He’s figured out that code review is the way forward in The Enterprise’s plans. He also knows that the key to his own advancement lies in being better than his peers at promoting strategic initiatives.
“How can we make sure that we’re on the cutting edge of The Great 2016 Code Review Initiative?” Ideas are jotted on a whiteboard over coffee and donuts. Before long, the plan is clear. Phase in code review by the end of Q2, and not Q3, and have two people review each line of code, instead of the one person mandated from on high. The secret sauce is in cutting non-essential meetings out of developers’ schedules in order to free up the time to justify the extra set of eyes. Office political brilliance.
But the best part of all is that this particular group of people recognizes the opportunity to solve a nagging compliance issue. By having two reviewers for each line of code, and having both reviewers sign off in an official registry of reviews, the program is now one step closer to bullet proof against audits.
And so it comes to pass that one particular project organization within The Enterprise adopts a modified, enhanced code review policy.
A Study of Motivations: Newly Minted Line Manager
In a small conference room with no windows and powdered coffee mix, a line manager recently promoted from the ranks of developers lords over his direct reports. Middle management, he informs them, has mandated a two-person signoff on each line of code to be reviewed. But he has something to add.
In all cases, he will be one of the two people performing the reviews. The official reason for this is that he has extremely deep knowledge of both the problem domain and the codebase, but the actual reason for this is that he does not trust his team. This is largely due to an abiding need to control his surroundings, and partially do to some culturally learned helplessness on the part of the team.
The team collectively shifts, sighs and shrugs, understanding that their flavor of The Great 2016 Code Review Initiative is, like so many things, a box to be checked that is completely beyond their control. They adopt the new policy apathetically, too put upon even to resent the addition of just one more box to be checked to their already full cognitive load.
As to the peer reviews themselves, they are rubber stamp affairs nearly entirely without substance, since the line manager will simply supersede them anyway. The team adopts a new normal in which they write their code, send it to a teammate who approves it with barely a glance, send it to their manager who demands dozens of stylistic changes, make the changes, and then call it a day.
Missing the Point
Somewhere in the middle of all of this, the point gets lost. Code review, as an activity, is a highly collaborative exchange designed to spread knowledge, improve skill levels, and improve quality. Other benefits will naturally follow, but only if the process is done by the team and not to the team.
All too often in the enterprise, policies like code review roll a long way downhill, picking up side benefits as first class goals. Code review becomes a cost savings measure, a legal cover, or vehicle for neurotic micromanagement. It becomes about everything but improving the code.
The most critical element that gets lost in the shuffle is trust of the development teams themselves. What if, instead of conceiving of processes that will fix the defect problem, leadership simply asked the teams to fix it, or at least to come up with ideas for fixing it? Any solution conceived of by the team in this situation would have much more substantial buy-in and be much more likely to work.
Code review, at the end of the day, is a tool that can be used by a team to help it improve the quality of its code. For this reason, it has to come from the team to be maximally effective, and they have to believe in its ability to help them solve their problems.
Independent of the size of the enterprise Code Review misses the point entirely if it is not backed up by tools that can objectively assess the code in the first place. I am going to assume that the “60% break/fix” number comes from statistics gathered slavishly from Support and is an approximation based on the number of non-duplicate “User Raised Issues”. Any CTO worth their salt would be asking “why do we have so many issues?” and then, being relatively current on technology, asking questions like “Why is QA not picking these up?” and “do our devs have a testing… Read more »
At companies the size of the ones in question, I think the C*s are involved with technical strategy at a level that could hardly be considered technical. “Why are we having so many defects” is a question delegated a few rungs down the ladder. “What’s the global IT budget” and “should we look at reducing headcount in favor of using more vendors?” QA and app dev are concerns at the program (or silo) level. As for code review, have you ever read Code Complete? This guy talks about the study McConnel sites regarding code review/inspection and its effectiveness: https://kev.inburke.com/kevin/the-best-ways-to-find-bugs-in-your-code/ I’ve… Read more »
I disagree about code reviews, especially about them being invented by a narcissist. I just introduced them to a company I consult with, and over the last 6 months, code quality has drastically increased, and the developers themselves are quite happy to have the review. Finally, last week, we were reviewing code that cannot be easily run tested (it talks to equipment that costs thousands of dollars per hour to use). And we found 3-4 innocuous looking bugs in a short review session. We do code reviews as PROFESSIONALS. We have coding standards which need to be applied. And 80%… Read more »
I did go on to say that Code Reviews do have a place when mentoring developers. Like any tool used correctly Code Review can be useful, rewarding and a powerful tool against technical debt. The way you have set your Code Review process up sounds pretty much how it should be done. I have seen it done both well and catastrophically badly hence my rather exaggerated “narcissist” comment; seen it done badly more often than well.