[webkit-dev] r- your own patches [was: Re: RenderArena: Teaching an old dog new tricks]

Yong Li yong.li.webkit at gmail.com
Fri Nov 16 07:08:06 PST 2012


2012/11/15 Ryosuke Niwa <rniwa at webkit.org>:
> On Thu, Nov 15, 2012 at 4:28 PM, Mike Lawther <mikelawther at google.com>
> wrote:
>>
>> On 16 November 2012 09:59, Ryosuke Niwa <rniwa at webkit.org> wrote:
>>>
>>>
>>> While I don’t want to further agitate the issue or go off on a tangent,
>>> and agree that we must address the security aspect before getting rid of
>>> RenderArena, only WebKit reviewers can r- patches written by other
>>> contributors. You’re not even supposed to set r- on your own patches. See
>>> http://www.webkit.org/coding/commit-review-policy.html
>>
>>
>> I see that page says 'Note that you should not put r+ nor r- on patches in
>> such unofficial reviews' with respect to a non-reviewer doing a shadow
>> review.
>>
>> I can't see the extrapolation from that to 'you can't r- your own
>> patches'. I thought r-'ing your own patch was a relatively common practice
>> when uploading a WIP patch, as a signal that 'I have no intention of landing
>> this patch', and as a courtesy so a reviewer will not waste any time looking
>> at it (unless specifically asked).
>
>
> r+ and r- flags are supposed to be set only by reviewers. If you wanted to
> withdraw your patch from the review queue, then you should be clearing  r?
> flag, instead of setting r-. If you’re uploading a WIP patch, then it should
> not bear either r?, r-, or r+ flags. You can accomplish this by either not
> setting the flag when you upload a patch on Bugzilla, clearing flag on the
> Bugzilla, or using --no-review option on webkit-patch.
>
> A patch with "r-" should be a patch rejected by a reviewer, not a WIP patch
> the contributor decided not to proceed with. Otherwise, reviewers can’t
> differentiate the patches rejected by other reviewers and patches authors
> didn’t like (unless you recognize author’s IRC nickname).
>
> - R. Niwa
>
>
> _______________________________________________
> webkit-dev mailing list
> webkit-dev at lists.webkit.org
> http://lists.webkit.org/mailman/listinfo/webkit-dev
>

Seconded. I also think only the one who submitted the patch can clear
the r? flag. Others should NOT do that, please, even you are a
reviewer. You can r- the patch if you believe it is bad.

-yong


More information about the webkit-dev mailing list