SeaMonkey code reviews and flag rules

Code reviews

Note that those rules apply to suite-specific code only. Toolkit has their own review rules, and so does core Mozilla code (Gecko, shared stuff, etc).

r= from module owner or peer (of all involved modules).
ui-r= is required for any significant UI change from an owner or peer of the UI Design module.
moa (can be requested via the sr flag but from the SeaMonkey MailNews module owner/peer) is required for suite/mailnews/ patches unless the SeaMonkey MailNews owner/peer says otherwise (this is for ensuring user data safety).
sr= is required if any of the patch's reviewers request it or the patch is a major API change or a major addition/removal of code.

Any module owner could theoretically request moa, which would mean that he is required to approve all patches in that module. That doesn't mean he actually has to review them, that can still be done by a peer or other reviewer approved by the module owner.

super-reviewers:

Neil Rashbrook

SeaMonkey super-reviewers don't necessarily have to be global Mozilla super-reviewers. If they are not, they only can grant that flag for SeaMonkey-specific patches with respect to the rules laid out in this document.

Bugzilla flags

All those seamonkey/comm flags referred to below can be requested (set to "?") by anyone, only SeaMonkey Council members are allowed to set them to "+" or "-" though.

tracking-seamonkeyXXX

Criteria for +
- we can't ship the relevant release without that issue fixed, and would push out the release for that
- security issues should be blockers in almost any case
- severe usability issues block Release; if the vast majority of testers can live with them, they might not block Beta, though
- high-profile crashes or hangs
- High severity issues and less severe bugs also block Beta or Release
  - Beta: agreement of one Council member
  - Release: if two aren't completely sure, need three to agree, preferably one of them being the release engineer
Criteria for - (anything meeting one of those can be minused without further discussion)
- minor glitches
- enhancements, feature requests (exceptions are when a Council majority thinks we need them for the release)
- things that weren't fixed in older final releases and did not harm there (as long as circumstances haven't changed radically)

We may end up in having blockers without an assignee. We (Council) need to make sure we get someone working on them, so we can get a release done.
Nice-to-have improvements that don't fix major problems don't block the release. Their patches might get approved, but the bugs get tracking- in almost any case.
We should try to keep the number of tracking+ bugs as small as possible. Remember that from the definition of the flags, the number of open blockers has to be zero to ship the release. We shouldn't slip that definition by far, ideally we should meet it.

approval-comm-XXX

Criteria for +
- low risk (Beta: very low risk, Release: minimum risk)
- no L10n string changes
- polish, stability improvement, or routine change (version number etc.)
- patch is ready and reviewed
- patch is tested well enough for the risk involved
- patch fixes failing tests
  - Beta/Release: agreement of one Council member or module owner
  - If the approval request was made by a Council member, the approval should be done by another Council member or the module owner
Criteria for - (anything meeting one of those can be minused without further discussion)
- L10n string changes (Beta/Release are string-frozen; exceptions like stability backouts on Beta prove the rule)
- high risk, low value

General advice: Developers should include risk estimation and level of testing the patch got when requesting approval.