In the previous two posts we saw what feedback can be given in code reviews, and how best to provide it.
However, reviewing every (or most) proposed changes in your team is time consuming.
In this final post I will try to convince you that the effort of rigorously reviewing code is worthwhile.
I believe that code reviews are worth the effort because they are a cost-effective way to instill a culture of high quality of work in a team.
A communication method for technical leadership
An important effect of strict code reviews is: It changes the culture of the team.
Every team has a culture – the set of (usually) unwritten, unspoken rules, that the team abides by. If the reason for a team’s processes or work method is “that’s just how we do it”, then it’s part of that culture. even if that “rule” was never explicitly stated, or even discussed.
Examples of this implicit culture can be –
“only QA people do testing”, “a developer never works on more than one task at a time”, “every change must be covered by unit tests”, “we never deploy on Fridays”, “it’s OK to release code that wasn’t tested if the product manager deems it urgent enough”.
These rules evolve organically as team members observe what team management values – Which behaviour gets rewarded, and which gets punished (or is not allowed).
And these implicit rules are much stronger than anything that is said explicitly:
How many times did you hear from management that “quality is our top priority”? How many times did you then go back to your desk and push out a hacky change because it was urgent?
Actions (in this case – allowing quality to suffer due to ‘urgency’) speak louder than words.
That’s why the review process is a powerful way to set a culture of high quality work. The action of rejecting low-quality changes speaks louder than any talk about quality.
Rather than talk about wanting to improve quality, we actively reject sub-standard work.
In my experience, developers pick up on that culture very quickly.
A developer may get irked when they are asked to break up a long method, standardaize a component’s API, or justify why a certain method belongs in a certain logical layer. Especially if they’re not used to these things being an issue.
However, after a few times, they will adapt. They’ll simply save themselves the time of submitting changes that are bound to be rejected.
Developers will follow those best practices not only because that’s the only way to get their work done.
But also because they’re interested in improving and leveling-up. And this review process, done with teaching and mentoring in mind, helps them do just that.
“Academic” Training can help team members recognize what constitutes high or low quality work. But how many times have you come back from training, full of wonderful new ideas, only to return to your familiar way of working in your old project?
The review process is an effective and efficient tool for making a high-quality way of work stick. This is because, unlike training, it’s baked right into the team’s processes, and it’s provided in the context of the actual work being done.
(pair programming is even better, as it allows teaching and discussion in real-time. However, it isn’t as time-efficient as reviewing code.)
Why you don’t do it – awkwardness
I sometimes ask colleagues why they don’t employ a review process as suggested in this series. By far the #1 reason I hear is awkwardness.
It isn’t that they disagree with the importance of it, or with the different guidelines; It’s the human aspect that stops them:
Conducting such a thorough review ends up with dozens of questions or requests for change. People feel uncomfortable doing that.
They worry that the code author would be insulted or made to feel inadequate by a torrent of comments.
These concerns are justified. That’s why the first post of this series is dedicated to ways of avoiding that. It’s important to ensure that feedback is provided well, and taken in context, and not as a confrontation.
I can relate my personal experience of employing this method:
At my job, I submit by far the most comments on any given PR (about 5 comments from me to 1 comment from anyone else).
However, I try to do so with empathy and sensitivity.
The feedback I receive is overwhelmingly positive.
My colleagues (and me) have a positive learning experience as a result of my questions / suggestions. They appreciate the thoughtfulness and effort that went into reading their work.
Remember that your colleagues, like you, value opportunities to learn and improve. Don’t be afraid to help them.
(In fact, if you’re in a leadership position, that’s probably part of your job!)
The role of senior leadership
Improving the team’s quality of work is one of the technical leader’s responsibilities.
If employing rigorous review is a way to achieve that, shouldn’t it be a part of every technical leader’s job? Just like coordinating the technical work on a project, or attending pointless meetings?
So why isn’t it?
Many, senior management included, regard low-quality work as an “act of god”. Something that we are powerless against. Or, at least, a problem that has so many different causes that nobody can be personally accountable for it.
The manager didn’t actually write that bad code. There’s nothing they could have done, right?
Well, they could’ve at least read that code..
Imagine explaining to upper management why certain features are delayed / buggy / unmaintainable. Imagine management asking “who authorised this low-quality of work?”
That would make not rigorously reviewing code an even more awkward situation for the technical lead. Thus helping her solve the problem of awkwardness around applying rigorous reviews :).
Series Summary
Some of the largest hurdles that teams face in trying to improve the quality of the code they produce are:
Lack of awareness (we don’t know that there’s a better way to do this),
Lack of skill (we know there must be a better way to do this, but we don’t know what it is),
or Apathy (we know there must be a better way to do this, but it isn’t important enough).
(The last one, often expressed as “time constraints”, is something that I don’t know how to solve )
As a leader (or team member), you can use the review process as a time-efficient way to address the first two issues.
Reviews are a tool to raise awareness of problems (by commenting on them). They also to help improve skill (by prompting for, or working together on, a better solution).
Most importantly – they signal that code quality is important enough, by not allowing low-quality changes to go through.
Reviews are more specific and relevant than training, as they deal with the team’s actual code. They are also less time consuming than pairing.
This allows a single person to influence greater quantities of written code.
Done right, code reviews are a tool for teaching, learning, and setting culture and standards. Give them a go!
The Useless Dev blog 