Adopting the mozilla-central superreview policy in comm-central

Justin Wood (Callek) callek at gmail.com
Thu Jun 10 23:59:19 UTC 2010


  Ok, I'm not inherently against this, BUT there are a few points I want 
to make.

The mozilla-central current rule exists after THOUSANDS of tests have 
been written and in use. And have (for a lot longer than us) REQUIRED 
tests for every bug patch.

Now I know TB has required tests, but we are still in a way "O, orange; 
must be a perma orange, likely not my fault" general mentality on trunk.

Suite and TB coverage here would help my level of confidence, even more 
so if we are sure to get a much larger and more redundant level of code 
coverage with our tests.

That said, additional reviews for the sake of additional reviews doesn't 
help much; I know that MANY of my changes are not api changes, and I 
will r? and sr? the same person when the section of code needs sr. 
Though I also will review others if I think multiple expertise will help 
the code itself, (say a build config thing that affects l10n as well as 
TB I'd of course r? KaiRo, and Mark). If it affects TB Buildbot, gozer; 
etc. but I don't feel a sr+ is required on many levels, based on my 
understanding of sr.

Second-Review could be a whole another story, and should be used if the 
reviewer doesn't fully grasp the code, etc.

-- 
~Justin Wood (Callek)

On 6/10/2010 6:01 PM, Karsten Düsterloh wrote:
> Mark Banner aber hob zu reden an und schrieb:
>>    Having thought about it on and off for a while now, I'd like to
>> propose that we adopt the mozilla-central super review policy for at
>> least the Thunderbird code in comm-central. The current policy is here:
>>
>> http://www.mozilla.org/hacking/reviewers.html
>>
>> I think this has various advantages, including not all patches in
>> mailnews requiring sr - especially the small ones where we tend to use
>> the same reviewers anyway - whilst ensuring that API changes get the
>> extra consideration that they sometimes need.
> I'm wary, to be honest.
> Especially /mailnews is a precious and in parts rather complex beast,
> which IMO has seen too few serious reviewing, not too much.
>
>> If we get general agreement here, I'm planning on taking this out to the
>> newsgroups and seeing if at least suite/ wants to join as well
> Note that /suite/mailnews even has a stricter policy than the rest of
> /suite, and I don't think this will change
> (<http://www.seamonkey-project.org/dev/review-and-flags>).
>
>> Please respond with questions, comments, etc. If there's no responses,
>> I'll assume everyone is in agreement and start moving the discussion
>> forward to the newsgroups.
> Actually, I find this policy change strange, to say the least. I'd say
> that TB needs - no offence meant! - more reviewing, not less.
> I'm aware that reviewers as such are a scarce resource, but (as I said
> before) you can't exchange competence by automation...
>
>
> Karsten
> _______________________________________________
> tb-planning mailing list
> tb-planning at mozilla.org
> https://mail.mozilla.org/listinfo/tb-planning




More information about the tb-planning mailing list