ITK/Release 4/Modularization/Code Reviews/Process: Difference between revisions
From KitwarePublic
< ITK | Release 4 | Modularization | Code Reviews
Jump to navigationJump to search
Line 12: | Line 12: | ||
Arbitrary texts, as detailed as the reviewer deem necessary. it could be paragraphs. | Arbitrary texts, as detailed as the reviewer deem necessary. it could be paragraphs. | ||
= Status | = 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 | 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. | * '''Pending''': No review has been performed yet. | ||
* '''Actionable''': 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 '''Actionable''': 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. | |||
* From '''Actionable''' to '''Done''': The assigned developer will communicate back to the reviewer when the actions have been taken, the reivewer will verify and if satisfied will set the status to '''Done'''. | |||
== Reviewer Actions == | |||
= Rating = | |||
* '''Good''': The file has been reviewed, and no change was required. | * '''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). | * '''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. | * '''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). | * '''Remove''': File must be removed (broke, obsolete, irreparable). | ||
= Essential Fields = | |||
REVIEWER=Name,STATUS=Status,ACTION=Action | |||
For example | |||
REVIEWER=Jim Miller,STATUS=Actionable,ACTION=Major | |||
REVIEWER=Alex Gouaillard,STATUS=Pending | |||
REVIEWER=Xiaoxiao Liu,STATUS=Done,ACTION=Minor | |||
REVIEWER=Brian Avants,STATUS=Actionable,ACTION=Remove | |||
= Workflow = | = Workflow = |
Revision as of 16:17, 15 February 2011
Overview
- 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 the following standardize content
STATUS: StatusKey (see table below) Arbitrary texts, as detailed as the reviewer deem necessary. it could be paragraphs.
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.
- Actionable: 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 Actionable: 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.
- From Actionable to Done: The assigned developer will communicate back to the reviewer when the actions have been taken, the reivewer will verify and if satisfied will set the status to Done.
Reviewer Actions
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).
Essential Fields
REVIEWER=Name,STATUS=Status,ACTION=Action
For example
REVIEWER=Jim Miller,STATUS=Actionable,ACTION=Major
REVIEWER=Alex Gouaillard,STATUS=Pending
REVIEWER=Xiaoxiao Liu,STATUS=Done,ACTION=Minor
REVIEWER=Brian Avants,STATUS=Actionable,ACTION=Remove
Workflow
Contractors
Refactoring
For classes to be refactored
the following process should be followed
- MANTIS Bug
- Dependencies
- Migration guide