Comments on Software Rants & Other Miscellany: Making Effective Use of Code Reviews

This is a great example of tailoring a process to ...

2011-04-12T09:35:00.755-04:00

This is a great example of tailoring a process to your needs instead of pulling a practice off the shelf and rigidly adhering to what the book says.

I think there's a pattern that sometimes emerg...

2011-04-11T13:03:04.100-04:00

I think there's a pattern that sometimes emerges where a code review drags on for way too many iterations. I think the reviewer thus has a responsibility to try to think of relevant feedback as early as possible, and to balance perfectionism with the goal of moving things forward.

Sadly, I've personally observed counterexamples of this where people were actually using code review to block progress intentionally.

Excellent article; this has been on my mind as wel...

2011-04-11T10:45:02.798-04:00

Excellent article; this has been on my mind as well.

I'd encourage you to follow up with something about code review antipatterns. Here are two opposing ones that I sometimes see:
1. The perfect is the enemy of the good: particularly working on a "mature" (messy) code base, there's a wide variance between what could be done and what should be done. The former entails lots of cleanup, writing a lot more tests, etc., but it's a boil-the-ocean process where there's a huge amount of work necessary before you can get anything done. The latter is the reasonable middle ground where changes are for the good and move the codebase forward without getting too entangled, and get positive changes made quickly. I find in some code reviews it's too easy for the reviewer to ask for a lot of rework; it takes clarity on the tradeoffs to push back with "That's a good idea, but too much for right now".
2. You can't really educate someone to be a better programmer via code reviews; the engagement depends on the engineer & reviewer speaking the same language and understanding the same goals. If the original coder doesn't really understand the coding quality guidelines, addressing specific review comments may bandaid over the problems, but it reduces to having the code reviewer fix the code via e-mail, or, eventually, giving up and letting code they're not thrilled with get checked in. This is particularly true when the issues aren't particular clear-cut but fall into qualitative evaluation categories, such as your points "complexity" and "maintainability".

Another best practice for developers sending out c...

2011-04-11T10:37:35.158-04:00

Another best practice for developers sending out code reviews is to keep the review as small as possible. It's much easier to review 5 200-line patches (assuming logical separation points exist) than a single 1,000-line one.

Small patches can also be a forcing function for better design (code is less inter-dependent). Ideally (D)VCs with easy branching like Git and Mercurial make this send-out-a-review-and-keep-going workflow easier too.