Code Review Guidelines

Table of Contents

• Introduction and Purpose
• Who Should Participate in a Code Review?
• Review Packet Preparation and Distribution
• Packet Alternatives – Electronic Reviews
• Code Reviewer Responsibilities
• The Review Meeting
• Problem Reviews
• Documenting The Review
• Code Review Checklists

---

Introduction and Purpose

The purpose of the Code Review is to provide a chance for peer(s) to inspect the work done for a particular OPR. The work that is reviewed includes:

• Software changes,
• Documentation – test instructions, release notes,
• Test Cases (for large or complex OPRs),
• Others…

By having peers review the work done for a specific OPR we hope to improve the quality of our software deliveries by:

• Finding bugs not detected by the developer,
• Verifying code meets team software standards,
• Ensuring documentation is well clearly written and complete,
• and in other ways.

Finally, the code review promotes dissemination of information about software changes and software areas by having other team members look at the work. This makes the review process not only a quality control process but an educational and training process.

Not all software changes warrant a code review. In addition, the size of the software effort under review will drive how the review itself is conducted.

---

Who Should Participate in a Code Review?

At a minimum, the code review should include one software developer. If the OPR is large or complex (either in implementation or it is particularly difficult to test), then two developers and a tester may be included in the review.

---

Review Packet Preparation and Distribution

In general most code reviews occur after all software work has been completed for an OPR. Sometimes however, a complex software project will require an iterative approach to reviewing the software.

The developer who is preparing the review packet should include the following items:

• OPR Description w/ requirements
• Listing of all new source code modules
• Listing of all modified source code modules. Changes should be highlighted in some manner. This can be done with a highlighted, the VMS DIFF command, or by some other annotation method.
• Test Instructions
• Release Notes

The review packet should contain page numbers – this will help the reviewers and you when the packet is discussed.

Finally, the packet should be distributed a day in advance in most cases. For large packets several days is appropriate. For small packets several hours can be OK if the participant can support it in that timely of a manner.

---

Packet Alternatives – Electronic Reviews

For small code reviews, a nice alternative to preparing the packet is to show another developer the modifications online. This requires a verbal explanation of the problem and a visual and verbal walkthrough at your workstation.

---

Code Reviewer Responsibilities

The success of the review is highly dependent on the code reviewers thoroughness and diligence in reviewing the packet. Most of the work is done ahead of time by individuals and not at the actual meeting. The code reviewer should watch out for the following things:

• Consistency – are modified sources done so in a manner that is consistent with the source file?
• Standards – are the software standards for C/C++/Fortran being followed?
• Bugs – Are the changes generally correct?
• Were the Requirements Well Understood?
• Documentation
  • Are all program headers completed?
  • Are changes commented appropriately?
  • Does documentation use Correct Grammar?
  • Any spelling errors should be corrected!
  • Are release notes clear? Complete? Understandable? Well Organized?
• Installation Issues
  • Are there any?
  • Are they clearly documented?
  • Is there any co-requisite OPRs listed?
• User Interface Issues
  • Are #NEWS messages clear? Are they missing any?
  • Are output products clear?
• Test Instructions – Are they clear? Complete? Understandable? Well Organized?

The code reviewer must produce a list of all major areas that need to be addressed by the developer. These should be commented into the OPR after the review as a statement of the hard work done by the reviewer and as a action item list of addressable items for the developer.

---

The Review Meeting

Before any meeting is held all review participants should have gone through the packet and made comments to bring to the meeting.

For large code reviews an actual meeting should be scheduled. The developer should conduct the review by walking through each major module describing the changes made and soliciting comments from the review participants.

For smaller code reviews the developer and review pair can meet informally and discuss the reviewers comments on the packet/changes.

---

Problem Reviews

One of the reasons that we take the time to perform code reviews is to prevent the wrong software solution from being delivered to our users. Consequently, sometimes a review will become a problem review, which means that there are too many areas that need to be reworked or cleaned up, and that the developer should re-review these changed modules at a later time.

Note: A problem review is a sign of a successful review process and should not be considered a failed software effort!

The reviewers should OPR comment the problem areas to be addressed by the developer and also document the fact that a re-review is desired.

---

Documenting the Review

All participants should document their review findings (either individually or in collaboration) in OPR system.

Code Review Checklists

---

Reviewer’s Checklist

 

• Requirements Reviewed and Understood?
• Test Instructions Reviewed and Understood?
• Release Notes Reviewed and Understood?
• Code Changes Reviewed and Understood?
• Is a Re-Review Needed?
• OPR Commented with Review Findings

---

Developer’s Review Packet Preparation Checklist

 

• OPR Printed
• Print Code or If Better, Difference Listings
• Highlight Code Changes For Existing Modules
• Get Packet to Review Team One Day Early
• Produce Table Of Contents With Numbered pages For Large Packets