G'day,
Edit: If you approaching the review as an exercise in "I don't trust you so I need to check what you're doing" then you're missing the basic reason for reviews and any review is likely to be less than successful.
An author's assumptions that their own code is perfect every time is, if not optimistic, then at least misguided.
People are human and they can, and will, miss various things in their code. A second or third pair of eyes will quite often pick up things that the author completely missed. Sometimes these things can be blindingly obvious and yet still missed by the author of the code.
There are several proven ways to ensure reviews are successful:
- As Neil B. pointed out, only team members involved in the code can be involved in the review. Unless of course there is good reason otherwise, e.g. the team is using a utility or maybe middleware written by another group and you want to verify your usage of the provided utility/middleware/etc.
- Emphasise that the project is a team effort and the team should be working together to ensure that the project, i.e. everyone's individual contribution joined together, is the best that they can do. It is not dev A's chunk which was better than dev B's chunk.
- Emphasise the learning possibilties available to the team from team members participating in the review. There cumulative years of experience of the typical software team is typically quite large. Getting all that experience pulling together in the same direction to make sure the best product goes out the door is a very powerful tool, especially for teaching the less-experienced of the team.
- Always emphasise that it is the code under review. Not the person.
- Always use language like "this function fails to test for negative inputs" and not "You forgot to check negative inputs"
- Do not allow long discussions/rejections/justifications in the review.
- There are only three responses possible during the review, accept the change, reject the change, take offline to discuss later.
- The code fragments must be provided before hand.
- All participants in the review, apart from the facilitator who is running the review meeting, must read the code beforehand.
- Time must be scheduled to allow reviewers to read through the code.
BTW You might like to look at Fagan inspections which takes the above points even further.
Edit: Sorry but I forgot to mention that you must have some coding standards in place. Otherwise your reviews will easily descend into "holy wars" with battles overthings like:
- brace style,
- TABs or no TABs in the source,
- indentation levels 4 spaces or 8 spaces
- etc.
Having a coding standard in place removes such discussions from the equation. Mind you you'll still have such, er, "exciting" discussions when making the coding standards! ;-)
HTH
cheers,