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