Peer Reviews / Code Reviews

We're big fans of peer reviews at RTSL.eu. We like the fact that peer reviews offer our teams the opportunity to a) apply additional quality control before we ship code out of our software factory, b) learn new techniques from each other, c) share knowledge about the applications we maintain.

In our context, a peer review is a review of an individual's work by a fellow team member.

We prefer the term Peer Review to Code Review because we need to review more than code. For starters, if we're using DI Studio, we need to review the DI jobs, not the code that it generates; and other SAS objects have no discerable code at all, e.g. information maps, but they still need to be reviewed. However "code" is a good shorthand for "all of the stuff that we make in our software factory".

If Peer Review is one of the last steps in our development process before the code ships out of the factory gates (to be passed to the testing team), we need to provide a quality stamp on all elements of what we're shipping. For us, peer review includes:

• Be sure the code matches the Programming Guidelines. We're not overly prescriptive with our guidelines, but we have to be happy that we can all support each others code, and that means there has to be a degree of consistency in how we code
• Be sure the code matches the Specifications. Peer review is static testing, so we don't execute the code, but we will check to see that the code appears to follow the Specs
• Be sure the changes made (if this is an update to existing code) match the Specs for the change. The developer will provide code comparisons that show what lines have changed between the current and the new versions. You'd be surprised how often this shows us that a developer has either i) omitted a part of the change, or ii) inadvertantly introduced an unintended change
• Be sure the developer has tested the code. The reviewer will expect to see test scripts and evidence of their successful execution
• Be sure the code and documentation are properly stored in our Configuration Management system. We want to be confident we know what we're shipping and that we can subsequently reference it for bug fixing, etc

Doing these checks need not be too time-consuming - there's no need to re-review anything that hasn't changed - but problems found in peer review are far cheaper to fix at this stage than if they were not spotted until later (in testing, or in production).

Crucial to the success of peer reviews is the fact that the reviewer is there to learn as well as offer constructive criticism. AND let me emphasise that the criticism must be constructive!

Roy Osherove's team leadership blog recently offered some pointed advice on peer reviews. It's a good read and offers sound advice on how to introduce peer reviews into your development team.