Re: sr flag question

2012-12-13 Thread Dave Townsend

On 12/12/12 19:06, Justin Lebar wrote:

Recently I read Dave Townsend's thread about "Super-review,
what shall we do with you?" and realized there wasn't any
conclusion to that.

As a relative new dev, I think it is vital to have a clear
distinction as to when a sr is required.


I think the conclusion to draw from that thread is that there is no
One True Policy about when SR is required.  We tried to make one in
that thread, but it was hard to reach consensus, as you observed.

Reviewers should point out when a patch needs sr.  Or you can flag
patches which you think might contain API changes for sr?someone and
then let the reviewer or superreviewer clear the request.


Yeah this is right. Reviewers should be the ones saying if they think a 
patch requires SR. They have the experience to understand that decision 
which is difficult to put hard criteria against. The words in the SR 
policy page can give you a general idea but circumstances differ.


If you are concerned about whether something will need SR then the 
things to do is reach out to a potential reviewer and ask them.


___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


Re: sr flag question

2012-12-12 Thread Justin Lebar
> Recently I read Dave Townsend's thread about "Super-review,
> what shall we do with you?" and realized there wasn't any
> conclusion to that.
>
> As a relative new dev, I think it is vital to have a clear
> distinction as to when a sr is required.

I think the conclusion to draw from that thread is that there is no
One True Policy about when SR is required.  We tried to make one in
that thread, but it was hard to reach consensus, as you observed.

Reviewers should point out when a patch needs sr.  Or you can flag
patches which you think might contain API changes for sr?someone and
then let the reviewer or superreviewer clear the request.

-Justin
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform


sr flag question

2012-12-12 Thread Edmund Wong

Hi,

Recently I read Dave Townsend's thread about "Super-review,
what shall we do with you?" and realized there wasn't any
conclusion to that.

As a relative new dev, I think it is vital to have a clear
distinction as to when a sr is required.  I've only done a
few patches that required sr (c-c stuff) and
I don't recall any m-c patches I did required sr (or
not that I know of).  But I feel that as I gain more
understanding of the code, I'll encounter the sr
flag more frequently. So I feel it is important that
a new dev understands clearly what the sr? flag is
and under what conditions it is required.  To veteran
devs it might be very evident, but it isn't for a
new dev, at least, not to me.

My understanding is that a sr flag is needed when there's
an API change or a UI change.  What constitutes an API change
or at least, to what extent would an API need to change in
order to require a sr?

For example, for the "EnumerateForwards/Backwards removal"
bug (bug #819940), would it have qualified as an API change
that required sr?  The bug didn't explain the rationale for the
removal so I'm assuming it was important or required.


Thanks for any clarifications,


Edmund


___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform