Calendar:Review Process: Difference between revisions
m (fixed wiki markup on link) |
(Revise this page, to reflect our new one-review approach.) |
||
| 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: | 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 | *The calendar/ code will be broken up into several smaller modules | ||
* | *At least one review will be needed for every patch | ||
** | **If a review comes from [[Calendar:Module_Ownership|the owner or a peer of the affected module(s)]] this review is sufficient. | ||
** | **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 | === The review === | ||
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 | |||
*fixes the identified problem | *fixes the identified problem | ||
| Line 18: | Line 16: | ||
*does not contain unjustified 'hacks' or 'work-arounds' | *does not contain unjustified 'hacks' or 'work-arounds' | ||
*conforms to the style of the surrounding code | *conforms to the style of the surrounding code | ||
For more information on what to look for in a | *is consistent with the rest of the module's code | ||
*is consistent with the direction/vision of the project | |||
For more information on what to look for in a review, see [http://www.mozilla.org/projects/seamonkey/rules/code_review.html these guidelines.] | |||
== Process feedback == | == 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. | 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. | ||
Latest revision as of 07:50, 6 May 2007
This document is intended to describe a refinement of the current code ownership and review process in calendar/. For a similar proposal for toolkit/, see this newsgroup thread.
Problem statement
Within Mozilla, code-ownership and the review process are closely tied. In order to maintain consistency, clarity, and quality in the code in CVS, the code owner and a small group of peers act as a filter for incoming code through the review process. The number of people involved in this filtering has, of necessity, remained small due to the need for each of them to be trusted by all to make similar/compatible decisions. This, however, has resulted in a rather long wait for reviews, which leads to frustration and slow patch development.
Review Process
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
- At least one review will be needed for every patch
- If a review comes from the owner or a peer of the affected module(s) this review is sufficient.
- 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 the owner or a peer of the affected module(s) is needed.
The review
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:
- fixes the identified problem
- gives no unintended errors or warnings
- is properly documented/commented
- does not contain unjustified 'hacks' or 'work-arounds'
- conforms to the style of the surrounding code
- is consistent with the rest of the module's code
- is consistent with the direction/vision of the project
For more information on what to look for in a review, see these guidelines.
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.