Calendar:Review Process: Difference between revisions

← Older edit

Calendar:Review Process (view source)

Revision as of 07:50, 6 May 2007

1,276 bytes removed , 6 May 2007

Revise this page, to reflect our new one-review approach.

Sipaq

287

edits

@@ Line 5: / Line 5: @@
 In order to try to avoid this bottle-neck, as well as to encourage interested contributors to rise within the Mozilla system, the review/ownership process is going to be changed, as follows:
 *The calendar/ code will be broken up into several smaller modules
-*Two reviews will be needed for every patch
+*At least one review will be needed for every patch
-**The first review may be done by anyone appropriate.  Ideally, it will be someone with with experience and ongoing interest in the relevant code.
+**If a review comes from [[Calendar:Module_Ownership|the owner or a peer of the affected module(s)]] this review is sufficient.
-**The second review must be done by [[Calendar:Module_Ownership|the owner or a peer of the affected module(s).]]
+**If the first review comes from a person, who is not the owner or a peer of the affected module(s), an additional review by [[Calendar:Module_Ownership|the owner or a peer of the affected module(s)]] is needed.
-=== The first review ===
+=== The review ===
-The first review is intended to be the chance for rising Mozilla developers to demonstrate skills necessary to become peers.
+When the reviewer signs off on a patch, it should be because he/she believes that the patched code meets the Mozilla standards. At a minimum the review should ensure that the code:
-When the first reviewer signs off on a patch, it should be because she believes that the patched code meets the Mozilla standards.  This is very much a learn-by-doing process.  While no exhaustive list of what a first-reviewer should look for can be given, at a minimum the review should ensure that the code:
 *fixes the identified problem
@@ Line 18: / Line 16: @@
 *does not contain unjustified 'hacks' or 'work-arounds'
 *conforms to the style of the surrounding code
-For more information on what to look for in a first-review, see [http://www.mozilla.org/projects/seamonkey/rules/code_review.html these guidelines.]
+*is consistent with the rest of the module's code
+*is consistent with the direction/vision of the project
-It is hoped that delegating these responsibilities will also allow contributors with limited time to develop small areas of expertise.
+For more information on what to look for in a review, see [http://www.mozilla.org/projects/seamonkey/rules/code_review.html these guidelines.]
-By delegating this responsibility to more members of the community, it is hoped that the review bottle-neck can be minimized.
-=== The second review ===
-The second review is intended to bring the expertise and knowledge of a module owner into code-analysis.
-This review must be done by [[Calendar:Module_Ownership|the owner or a peer of the module(s) patched.]]  While the second-reviewer will also examine elements of the first-review, a strong first-review will ensure that this takes little time and few, if any, iterations.  Instead, he can focus on ensuring consistency within the module's code and intended vision/direction.  Note that a throrough job on [[Calendar:Development_Strategies|Step 2 of the Basic Bug/Feature Workflow]] will help ensure this part of the second review goes smoothly.
-In addition, the second reviewer should provide feedback to the first-reviewer on ways he can improve future reviews.  Through this feedback process, it is hoped that new peers can be more quickly identified and brought more fully into the development process.
 == Process feedback ==
 This process is not intended to be set-in-stone.  Should you notice problems or ways to improve this process, please bring them to the attention of the calendar developers.