changes in Firefox review policy

[Mike Shaver]

I just updated the Firefox review policy, chiefly to

* update the list of active reviewers; and
* eliminate the now-cumbersome submodule system in favour of broad
reviewer responsibility and reliance on their judgment.

You can read the page at https://wiki.mozilla.org/Firefox/Code_Review
and I've copied the text below as well.

** Reviewers **

Firefox reviewers are the people who make sure that Firefox is
awesome, and that patches do no harm. As in other modules, reviewers
in Firefox might not have expertise in all aspects of the Firefox
code, but any Firefox reviewer can review patches in any part of
Firefox if they feel that they are able to do so appropriately. (For
purposes of Mozilla processes, such as vouching for commit access, all
Firefox reviewers are considered peers.)

Reviewers in the Firefox module are expected to:

know their limits, and decline or (preferably) redirect to those
more qualified as needed;
have interest, if not expertise, in all aspects of Firefox;
speak their minds clearly but considerately;
support the decisions of their peers, though they need not censor
their own polite disagreement;
participate in the development of Firefox contributors;
represent Firefox architecture and implementation issues to other
groups or projects; and
look for opportunities to improve Firefox as software, a product,
and a community.

Any person with level 3 commit access can become a Firefox reviewer
given the recommendation of 3 other reviewers.

** What does r+ mean? **

Or, "what to expect when you're granting a review".

Review being granted in Firefox means:

I don't believe that this patch does harm. Areas of focus include:
security concerns, especially where content and chrome touch each other;
significant performance issues, especially on key paths like
startup and page load;
localization and accessibility issues.
If it does cause problems, I will help make it right.
The patch has test coverage appropriate to the change.
Saying Thank you for your patch!

Review in Firefox is not conditional on:

The patch being the best possible way that the reviewer can
envision of achieving its aim.
The patched code reads as if it were written by a single person.
Style should be consistent, but it need not be identical.

** Making a good patch **

There are some things that make it easier to get a good, prompt
review, and to improve the quality of the patch you submit. Help your
reviewers help you, and don't be surprised if you get asked about one
of the below. (Don't be ashamed either, this stuff is complicated and
everyone forgets now and then.)

small patches, functionally divided;
description of any security concerns, especially where content and
chrome touch each other;
significant performance issues, especially on key paths like
startup and page load;
localization and accessibility issues;
some tests;
descriptive commit messages including your name for attribution.

** Help, my patch isn't being reviewed **

The reviewer list might look long, but these are busy people and there
can be a lot of patches to review. Sometimes patches fall through the
cracks, for which we are all quite sorry.

A poke in the bug can help, or direct email to the designated
reviewer. If that doesn't work, please mail Gavin and Dietrich for
assistance.

Mike

[Mike Shaver]

On Thu, Sep 29, 2011 at 12:31 PM, Mike Shaver <mike....@gmail.com> wrote:
> I just updated the Firefox review policy, chiefly to
>
> * update the list of active reviewers; and
> * eliminate the now-cumbersome submodule system in favour of broad
> reviewer responsibility and reliance on their judgment.
>
> You can read the page at https://wiki.mozilla.org/Firefox/Code_Review
> and I've copied the text below as well.

There are some grammar mistakes in there, but I figure they're Gavin's
problem now.

Mike