[webkit-dev] webkit-patch and clearing flags

[Bem Jones-Bey]

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)

Or do we also assume that #2 can't happen when looking for reviewed-but-not-yet-landed patches?

(There is a variant of #2, where the patch with an r+ hasn't been marked as obsolete, but there is a newer patch uploaded, but I think it would just add noise to the current discussion, so I didn't include it, along with other corner cases that one could invent.)

- Bem