Reviews are Evil

Posted by Keith McMillan

July 24, 2008 | Leave a Comment

I’ve thought for years that many reviews were just poorly done, and like a wayward child, could maybe be rescued.  I’ve changed my opinion, reviews, as practiced today, are pretty much beyond redemption, and the do more harm than good.

Almost every company I’ve worked for has practiced reviews after the fact.  The reviews seem to me to serve two different purposes, the first being to present a topic so The Experts can see what you’ve done, and pass judgement, and the second being so that other stakeholders can be informed what you’re about to do to them, er, I mean so they can see what you’ve done for them.   Let’s talk about each of these separately.

The first is the review so The Experts can pass judgment on what you’ve done.  In this scenario, someone would go off and do some design work, or produce some source code, and then convene a review.  Sometimes they send out what they want reviewed before hand, and other times they wait to pass out the material to be reviewed until the meeting.  In either event, it doesn’t particularly matter.  Usually by the time you want do to a review, the real work has already been done, and the most you can do is to make cosmetic changes.

The best instance of this type of review I’ve seen is one company that sent out the stuff to be reviewed, along with a feedback form, which was used to record specific comments.  The reviewers in this case were marked “optional” and “mandatory” and the person getting the review was responsible for getting feedback from each of the mandatory reviewers, even if the feedback was “no changes.”  If anyone had suggested changes, a review meeting was held, and the suggestions were addressed.

This approach at least tried to ensure that people looked at the material they were reviewing, but it still failed to address the fundamental concern: this type of review is too late to really make a significant difference, and tends to devolve into an exercise in fixing minutia.

The intent here can be noble.  If you have people who are newer and less skilled, or if you just want a second set of eyes, then this can seem like a good idea.  Typically, the amount of things to review mean that it’s impractical in fact, unfortunately.  If you want to get a second set of eyes, you need to do it regularly and much earlier in the process.  Code swapping or pair programming are better techniques to do this than the review.

The second type of review is the “here’s what we’ve done for you” review, at which you call together the stakeholders such as the customer or the other influencers in the organization and go over what’s coming to get their feedback.  This is usually a shallower review than the previous type, but again tends to occur much too late in the process.  To be successful in developing software, you really need much more regular input than this approach provides. Worse yet, getting the reviews you need slows you down more than it really should.

I used to think that we could somehow change these reviews around to minimize their bad aspects (delays, feedback that’s too late to be effective, wrong level of detail, etc) but I think we’d be better off just starting over, and finding a different way to serve the needs.  In a development team, you can practice pair programming or code swapping.  When I worked at Lontra, all my developers worked in all the code, so we all knew what each other’s work looked like.  Code reviews were informal, held at your desk to just get another quick set of eyes to see if you did something stupid.  It seemed to work out well.

Reviews for stakeholders should really be previews: here’s what we’re going to do for you, or what we’re currently doing for you.  Much better to get that feedback earlier, when it can make a real difference, instead of late, when the work’s already done.


RSS feed | Trackback URI

Comments »

No comments yet.

Name (required)
E-mail (required - never shown publicly)
Your Comment (smaller size | larger size)
You may use <a href="" title=""> <abbr title=""> <acronym title=""> <b> <blockquote cite=""> <cite> <code> <del datetime=""> <em> <i> <q cite=""> <s> <strike> <strong> in your comment.