On Fri, Nov 16, 2012 at 7:16 AM, <noam.rosent...@nokia.com> wrote: > > From: ext Ryosuke Niwa <rn...@webkit.org> > > 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. > > Regarding WIP patches, what I've seen a few times is us reviewers adding > an r- flag to a WIP patch with no r?, when we think it's horribly wrong… > I think the flip side of the guideline for non-reviewers to avoid r- is to > have reviewers use r- only when the patch is up for review. This will > encourage people to use no flags instead of putting r- for WIP patches. >
Yeah, I agree that's misleading as well. On Fri, Nov 16, 2012 at 7:42 AM, Julien Chaffraix < julien.chaffr...@gmail.com> wrote: > > 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. > > I disagree with that. You seem to think that patches falls into either > good or bad. However the reality is more complex and there are levels > of goodness and badness. I use r- for patches that I really think are > not in the right direction or shouldn't be landed: it is a statement > in this direction. Clearing the flag is for patches that are close > enough but still not up to our standards and that I want to kick off > the review queue. > I don't think reviewers should be clearing r? flags unless the patch has become obsolete (and/or the author is no longer actively working on it) and we want to get it off of the review queue. I would leave a comment and leave r? or I would r+ it with comments when I feel a patch is at the borderline condition. If you believe the patch should not be landed as is, then you should just r- the patch. If anything, the author can set r? flag back after arguing against your objection. - R.Niwa
_______________________________________________ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev
