[webkit-dev] webkit-patch and clearing flags

[Ryosuke Niwa]

On Wed, May 29, 2013 at 11:21 AM, Bem Jones-Bey <bjonesbe at adobe.com> wrote:

> On May 29, 2013, at 10:09 , Darin Adler <darin at apple.com> wrote:
>
> > On May 29, 2013, at 9:21 AM, Bem Jones-Bey <bjonesbe at adobe.com> wrote:
> >
> >> Would it be reasonable for webkit-patch to not clear flags on an
> attachement when it obsoletes it if there's an r+? Maybe it doesn't
> actually matter (i.e. I know I can still commit the patch), but it bothers
> me when it clears away an r+ because I forgot to tell it --no-obsolete when
> I'm updating a patch for nits.
> >
> > Yes, we should make some change like this.
> >
> > The problem is that it complicates writing a query that looks for
> reviewed-but-not-yet-landed patches. We need to solve that problem too.
> >
> > It’s valuable to keep the history of what has been reviewed.
> >
> > It’s valuable to conveniently find patches that have been reviewed but
> not landed.
>
> I don't see why it would make it more complex than the current situation.
> Right now, you have the following important cases for a patch that is
> reviewed but not yet landed:
>
> 1) the patch has an r+ flags, and is not obsoleted by a newer patch
> 2) the patch got an r+, and has been obsoleted by a newer patch for nits.
> 3) the patch got an r+, but it was cleared by webkit-patch and marked as
> obsolete, and there is a newer patch for nits
>
> I'm suggesting we make it that #3 can't happen, make webkit-patch not
> clear the flags when it marks a patch as obsolete, but only if it got an r+
> (clearing the cq flag would still make sense here, though)
>

Instead of doing this, webkit-patch should give the user two options:

   1. Clear r+ from and obsolete the old patch, and upload new patch with
   r?.
   2. Clear r+ from and obsolete the old patch, and upload new patch after
   filling out the reviewer names with cq? or cq+.

- R. Niwa
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-dev/attachments/20130529/6f53c543/attachment.html>