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

[Maciej Stachowiak]

On Nov 15, 2012, at 4:56 PM, Ryosuke Niwa <rniwa at webkit.org> wrote:

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

Personally I think it is ok to r- your own patch (it doesn't really violate the spirit of the rules about setting the review flag), but it's probably better practice to unset the review flag for the sake of clarity.

Regards,
Maciej
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-dev/attachments/20121115/66a7263f/attachment.html>