ITK/Release 4/Modularization/Code Reviews/Process

This page summarizes a process proposal for managing the code reviews of the modules in ITK.

Potential Implementations

1. Create a Dedicated MANTIS instance for the project "ITKv4Review"
2. Create a Dedicated Git repository for the project "ITKv4Review"
3. Create a Dedicated Trac instance for the project "ITKv4Review"

In each one of these cases, we could adopt the review workflow described below

Overview

In this case we describe the process assuming a Git-based implementation.

• Kitware will create a separate Git repository containing
  • One text file for each one of the files in the current ITK Manifest.
  • The files will be named after the ITK files. For example
    • itkImage.h.txt will be the code review file for itkImage.h
• The repository will contain at the top level, one directory per contractor, and under that directory we will replicate the monolithic directory tree structure of ITK with only the files that have been assigned to that contractor.
• This text files will be populated by the code reviewers with comments describing their observations during the code review process.

Workflow

This section describes the life-cycle of a file when going through the code review process.

Two concepts are introduced here:

• Status: the current state of a given file in the code review process
• Rating: the severity of the modifications needed by this file

Graphical Overview

Review Status

For each file, a contractor will review the code following the Check list (reference here) and specify one of the following potential status

• Pending: No review has been performed yet.
• Rated: Review has been performed and it resulted into actions to be performed by a developer.
• Done: Corrective actions have been taken. No further action required.

Transition Rules

• All files start in the Pending state.
• From Pending to Rated: A reviewer will review the file following the CheckList (reference here), and identify actions to be taken, and will assign a developer to look into the details. The reviewer will assign a rating according to the list below.
• From Rated to Done: The assigned developer will communicate back to the reviewer when the actions have been taken, the reviewer will verify the changes and, if satisfied, will set the status to Done.
• From Pending to Done: A reviewer may make this transition if the rating is Good as defined in the rating section below.

Rating

• Good: The file has been reviewed, and no change was required.
• Minor: Minor typos. In this case the reviewer will fix the problems directly (and still submit them to Gerrit). Minor changes are defined a those that do not influence the API. (including doxygen documentation).
• Major: File needs refactoring or major bug fixing. The reviewer must log a mantis and include the Bug number in the comments in the .txt review file, and the developer to which the bug has been assigned.
• Remove: File must be removed (broke, obsolete, irreparable).

Refactoring

For files to be refactored, that is, files that were rated as:

• Major
• Remove

the following process should be followed.

• The reviewer will log a MANTIS Bug (in the standard ITK Bug tracker)
• The reviewer will assign this bug to the developer who may be more knowledgeable about the file, e.g. the first committer of the file).
• From there, the bug will be managed in the standard way.
  • Dependencies with other changes will be discussed at this point
  • The assigned developer will discuss fixing alternatives
  • Will implement one of such alternatives
  • The developer will push a patch to Gerrit
  • Any API issues will be managed following the Migration guide documentation procedure
  • The patch will go through code review
  • Eventually will be merged into master
• Once the fix is merged, the bug will be closed, and the developer will notify the reviewer
• The reviewer will mark the issue as "Done" (using the adopted implementation: Git, MANTIS or Trac).

Potential Implementations Details

MANTIS

The following describes the potential implementation of this workflow by using MANTIS as a tool.

• A separate MANTIS instance will be created, specifically for ITKv4 Review
• The Status field will be created (Pending, Rated, Done)
• The Rating field will be created (Good, Minor, Major, Remove)
• A MANTIS issue will be created for every single file (done by a script)
  • The reporter will be the contractor (done by a script)
  • The reviewer will set the Status and Rating fields according to her review.
  • Some of these issues may have to be mapped to the MANTIS issues in the standard ITK bug tracker

Git

The following describes the potential implementation of this workflow by using Git as a tool.

• A separate Git repository will be created (re-using the existing accounts), specifically for ITKv4 Review
• The repository pre-populated (via scripts)
  • It will have one directory per contractor
  • Inside that directory the ITK directory structure (monolithic) will be replicated
  • Inside those subdirectories, the files that are assigned to that contractor will be copied
  • The files will be text files whose name matches one-to-one the filenames in ITK, plus the extension .txt.
    • For example, there will be a file itkImage.h.txt for the reviews of the file itkImage.h.
• The presence of the empty .txt files will indicate a Pending status
• Reviewers will edit those files, as they review the ITK code, and will populate the files with the ratings and any additional comments that they consider relevant. The changes will be committed, and merged to the master branch of this dedicated repository.