ICS 125: Design and code reviews

A review is a development activity where team members carefully, methodically look at requirements, designs, UI, code, or other artifacts.
A review is usually a meeting, although the trend is toward doing reviews more on-line.
Two major types of reviews disucssed in the literature:
- Walkthrough: less formal
- Inspection: more formal
- These will be discussed more below
- You can adjust the method to fit your project
Two types of issues that reviews help find:
- Implementation issues: e.g., coding errors, missing test cases, poor coding style
- Validation issues: raise questions about what is really desired, and what is feasible

The main goal of reviews is to find defects
Reviews are a good compliment to testing to help assure quality
As compared to testing, reviews can often:
- Find certain defects that would not be found in testing
- Find certain defects sooner than testing
- Find certain defects with less effort testing
Other important goals of reviews:
- Keep developers familiar with the work done by other developers
- Align expectations with reality
- Foster SPI: e.g., build checklists
- Generate partial defect metrics early

Two main types of reviews
Walkthroughs: less formal
- Participants make lists of questions and probable defects
- Participants meet and go over lists
- They discuss each item on each person's list
Inspections: more formal
- Participants meet and go over document/code line-by-line
- Participants "play computer" or discuss correctness proofs
- They use checklists/experience to identify potential problems
- Produce report of defects found
- After defects are fixed, produce another report describing resolutions
Common features of both
- A handful of participants: 3-6
- A review meeting takes a little planning ahead of time
- Participants must study document/code independently before meeting
- The meeting is to identify potential issues
- Meeting time is typically limited to at most 2 hours per day
- Limited time means that the documents/code to review should be carefully choosen
- All issues identified must be tracked to completion
Also useful
- Personal review of one's own work. Schedule time for it and track issues.
- Buddy review (only 2 people). Frequent small meetings.

Requirements reviews:
- Led by product management
- Participants from every affected department
Design reviews:
- Led by QA team member
- Participants from development team and QA
Code reviews:
- Led by QA team member
- Participants from development team and QA
All reviews:
- It is important to have some experienced people
- Junior team members can learn and contribute too
- Management should get report, usually not participate directly
- Reviews are not intended to evaluate employee performance

Early and often
- "Early" saves money because less is built on top of faulty code, requirements, or design
- Early finding helps estimate number of defects before too late
- Early fixing helps estimate effort to fix before too late, creates and uncovers
- Often helps catch individual defects before they can interact, or mask each other
Specific times
- Regularly scheduled: e.g., pick a few things to review every week
  - this is my recommendation
  - use other criteria to select review items
- Milestone-driven: Set review goals for internal milestones
- Change-control: Buddy/review all changes to maintence branches
- Risk-driven: Use reviews to manage identified risks
When not to work on reviews?
- When you dont know what to look for. You need better requirements.
- When you find too many defects. Go back to the requirments and design and find out what went wrong
- When you are getting tired of remembering rules. Take time to find/write/update checklists or automation.
- When too many major defects are found but not yet fixed. Current reviews will be invalidated by all the changes

Tools for reviews
- Style guides: coding style, UI style, e.g., Sun's java style guide or PHPBuilder.com's PHP style guide
- Checklists: ask probing questions, list recurring concerns.
- Recently discovered defects in other areas: they could occur here too
- Static analysis tools: lint, lclint/splint, jlint, checkstyle, Jcsc, also try setting compiler warning level options
Tips on successful reviews:
- Try to do most work through an issue tracker
- Make the review process "not personal" by relying on checklists, tools, guidelines that apply to everyone equally
- Focus attention on key aspects or risks: e.g., security, efficency
- DO NOT try to solve issues during the review, that happens independently
- Defects are not the only outcome: also build shared understanding. Write it down in report, documentation, comments, or assert statements.

Books on "peer reviews", "inspections", "walkthroughs", "quality", or general software engineering books
Websites: ProcessImpact.com, tigris.org
Any of the tool mentioned above