ITK/Release 4/Modularization/Code Reviews/Process: Difference between revisions

From KitwarePublic
Jump to navigationJump to search
No edit summary
Line 20: Line 20:
*** itkImage.h.txt  will be the code review file for itkImage.h
*** 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.
* 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 the following standardize content
* This text files will be populated by the code reviewers with comments describing their observations during the code review process.
 
  STATUS: StatusKey (see table below)
 
  Arbitrary texts, as detailed as the reviewer deem necessary. it could be paragraphs.


= Workflow =
= Workflow =

Revision as of 19:45, 15 February 2011

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

Graphical Overview

This is a graph with borders and nodes. Maybe there is an Imagemap used so the nodes may be linking to some Pages.

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 classes to be refactored

the following process should be followed

  • MANTIS Bug
  • Dependencies
  • Migration guide

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.

Trac