On Code Reviews... again
I gave a presentation on code reviews to the Philly CFUG a few weeks back. Then my host died, and I never posted this. I'll try and get the presentation up on the site in the next few days. But two questions arose that I figured I would discuss here:
- Do you check the code after the review to make sure developers have made the changes for which the team has asked?
- Can developers request a code review?
Both of these speak to a subtler issue. Does your team want to be adequate or excellent? What about each member, what do they want? The answer to this question will inform the answer to the previous ones.
Now this isn't want of these things where I'm using reverse psychology to encourage what I want others to do --- "Well if you want to be only adequate..." Look, not everyone can be excellent. I don't want to get into a whole discussion on where someone brings up "Free Markets" and someone brings up Harrison Bergeron and then we all get angry at each other. My brief statement on this is that there is plenty of value contributed by people who are only adequate.
But it's going to make a difference with a whole lot of things, including code reviews. Those that want to be excellent welcome criticism, because the idea of their product being flawed in any way is appalling to them. It's more appalling than letting other people find those flaws. But really, they like to show off what they've done, and want to be acknowledged by their peers for it too.
On the other hand, those that only want to be adequate don't care if something has flaws, as long as it works. They may try and avoid reviews, but mostly they'll tend to be indifferent to the whole process, and will only do it if they are mandated. Unfortunately once it's mandated it becomes a perfunctory step for them, a hoop to jump through. Some of the more militantly adequate will say things like "Don't bother fixing it; let's see if we can sneak it past the code review."
So, if you have group of developers striving to be excellent, they absolutely will ask for their own reviews (but you might need to let them know they are available) and you won't have to check them afterwards. On the other hand, if your team is on the other end of the spectrum, you might just have to mandate them. Perfunctory code reviews while painful are better than none at all.
As for double checking the code, it comes down to trust. Do you trust your teammates? I say err on the side of trust. Code reviews tend to ruffle a few emotional feathers to begin with. To big brother people afterwards would just add insult to injury. Trust people until they give a reason not to trust them. Obviously if you are doing work with sensitive data you may want to evaluate this on your terms.
But if people have violated your trust, you're going to be glad you followed the rest of my advice on code reviews. That way you'll have page names and line numbers by which to judge whether or not they kept up their end of the bargain.
This tension between adequate and excellent effects many other aspects of your team. I'm thinking you may want to look at this for more than just code reviews. I figured this would be a good place to sneak it in.
