Calendar:Review Process

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
• Two reviews 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.
  • The second review must be done by the owner or a peer of the affected module(s).

The first review

The first review is intended to be the chance for rising Mozilla developers to demonstrate skills necessary to become peers.

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
• 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

For more information on what to look for in a first-review, see these guidelines.

It is hoped that delegating these responsibilities will also allow contributors with limited time to develop small areas of expertise.

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 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 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.