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

[Ryosuke Niwa]

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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-dev/attachments/20121115/72af9a1f/attachment.html>