Freigeben über

Motley says: "Inspections suck... time that is"

  • Artikel

  • Summary

  •  

  • Motley:  Inspections seem a bit heavy. They suck up too much time for little gain.

  •  

  • Maven:  Inspections are actually a fairly light-weight structured process that find more issues compared to ad-hoc peer reviews and team reviews.

  • ______________________________

  •  

  • [Context: Maven and Motley are continuing a conversation about code reviews, now with a focus on inspections]

  •  

  • Maven: Nice restaurant choice, Mave. Although I am a little worried about what this Mexican food will do to me this afternoon.

  •  

  • Motley: Let's get this conversation over with quick so I don't have to be around you this afternoon! You were saying something about "injections" or something? Hopefully they aren't lethal!

  •  

  • Maven: Inspections. Inspections are a structured team-based review process for code, design, requirements, and any other artifact. Inspections are much more effective than ad-hoc peer or team reviews. The process is very simple - six steps, of which 2 are optional. Here is the overview:

  •  

  •  

  • Motley: That still looks pretty formal and heavy to me! Just the fact that you have a diagram describing all the steps is a little scary. I'm worried this practice is going to suck up a lot of time.

  •  

  • Maven: Ah, don't be afraid. The process is actually quite simple and not unlike other code reviews. It's just a little more structured with a couple of key practices to ensure that reviewers are highly effective.  First we start with planning.

  •  

  • Motley: Yeah, the process is so complicated I bet you need a plan just to pull it off!

  •  

  • Maven: Please! Put that chimichanga back in your mouth. Planning is really just common sense. You figure out what you are going to review, who should be involved, and plan the meeting(s). Not much to it. Next comes the overview.

  •  

  • Motley: You mean the 2 hour overview meeting where you are going to describe this process to everyone?

  • Maven: Always a wise guy. The overview is actually optional. You want to ensure that everyone involved in the inspection is sufficiently familiar with the work item to be inspected to do a good job. An overview is an ad-hoc meeting that usually doesn't last more than half an hour. If you are doing a code inspection, the author of the code gives an overview of what the code is doing (i.e. the requirements) and how the code was designed. There may also be a discussion of areas to watch out for, assumptions that were made, and it's a chance for inspectors to ask questions. Next, we prepare for the inspection.

  •  

  • Motley: What's the point? Shouldn't we just get together in a meeting and review the work?

  •  

  • Maven: Preparation is actually the most important part of the process. Here, each individual inspector takes some personal alone time to scour the material for issues. With code, an inspector looks for bugs, maintainability issues, and anything else a quality-oriented developer would commit to fix. Just getting together in a meeting to review the work on-the-fly is not effective. The real issues are found during individual preparation. After everyone prepares, THEN you have the meeting and collect the issues from everyone.

  •  

  • Motley: Hmmm… hopefully that meeting is extremely quick! If everyone does most of the work on their own, then we won't need much time for the meeting. In fact, why do the meeting at all?

  •  

  • Maven: The meeting is quick, but it is important to discuss some of the issues with the author. Treat it as a big learning experience for everyone. Plus, some of the things that people find may need clarifying questions and may not actually be issues at all. The meeting is a chance to validate that everyone has prepared, collect some metadata about the meeting (such as meeting time, number of issues found), and learn from one another. After the meeting is over, we take a few minutes to talk about the root cause of some of the issues that were found, and try to address them such that we don't make the same mistakes in the future. After the meeting, some rework is usually necessary.

  •  

  • Motley: Well, duh! If you find bugs you need to fix them.

  •  

  • Maven: Exactly, provided we are not at the point in the development cycle where only the really major stuff gets fixed. In any event, after most reviews, the author goes away and follows the processes of the team to fix the identified issues. After that happens, there may be some follow-up.

  •  

  • Motley: Because we don't trust the author to make the correct fixes? That's babysitting!

  •  

  • Maven: It depends on the developer. This step of the process is another optional one. We may want to have someone follow up if the developer responsible for the code is junior or new to the company. Another reason for follow-up, and even re-inspection, is if many major issues were found.

  •  

  • Motley: And what's this process improvement thing down the side of the diagram?

  •  

  • Maven: Let's save that discussion for another time.

  •  

  • Motley: So I bet your next request is that I just go off and try this. Fine. I'll humor you and give it a shot on some code. I'm not convinced it will help us, but I'll give it a shot.

  •  

  • Maven: Great! We can pick some code to inspect this afternoon and get it going.

  •  

  • Motley: Hold on, Burrito Boy. I don't want you anywhere near me this afternoon. I'll try it out with the team and let you know how it went.

  • ______________________________

  •  

  • Maven's Pointer:  The lean way of thinking is that inspections should not be necessary if we build quality into early parts of our development. Unfortunately this is just not feasible with software and other domains due to the human element. Think of inspections of code as similar to an inspection of your house under construction. Someone does the inspection that knows the domain, knows what to look for, and they make it a mandatory part of the house construction process. It is worth the small time hit to ensure a quality deliverable.

  •  

  • Maven's Resources: 

    • Introduction to the Team Software Process, by Watts Humphrey, Addison-Wesley Professional, ISBN: 020147719X, August 1999.
    • Software Inspection, by Tom Gilb and D. Graham, Addison-Wesley Professional, ISBN: 0201631814, December 1993.

Comments

  • Anonymous
    October 31, 2007
    Interesting variation on the theme so far.  However, I only count two optional steps on the slide, not four.  It looks like four are mandatory (or why are we doing this?).

  • Anonymous
    October 31, 2007
    Sorry about that - typo in the text. I have now fixed it. You are correct, two steps are optional, not four.

  • Anonymous
    April 21, 2009
    Summary Motley: There is no harm shortcutting a few steps of development to meet a deadline. Some up-front