[webkit-dev] Review Queue

[Kevin Ollivier]

Hi Eric,

On May 21, 2009, at 9:47 PM, Eric Seidel wrote:

> Interesting analogy.  However, closing means to me that the community
> is done with the bug.  Denying a patch because no one's working on it
> anymore (aka, no one is there to respond to review comments even if
> you make them) is not the same as closing a bug.  There is a
> "forgotten patches" link on the nightly start page iirc which shows
> all the r-'d patches. :)  http://nightly.webkit.org/start/  I've
> certainly looked through that list for patches to finish before.
> Maybe I'm the only one.
>
>
> Many of the bugs we see languish in the review queue are just too big
> to be easily reviewed.  I don't think we encourage enough decisive
> action from reviewers (like just r-ing a patch because it's too big to
> review).  At least an r- action gives the contributer something more
> than silence. :)
>
> This could of course all be avoided if we got our review queue down to
> something manageable. :)

My issue with this is that doing r- doesn't really solve the problem,  
it just makes the patch magically seem to disappear from the review  
queue. In general, I think it does make sense to prune really old  
patches and bugs periodically, but 2 weeks is way too fast IMHO (and  
even 1 month probably), even considering bit rot, and what I don't  
understand is why you want to put the burden on patch submitters to  
resubmit because their patches aren't getting reviewed? I don't see  
how this helps patches to get reviewed; in fact, quite the opposite -  
they may magically disappear from the queue on just the day someone  
might have decided to check the queue and actually review it.

I do understand the frustration, as I've had my fair share of patches  
sit for a while awaiting review, but from what I can tell all your  
changes will do is just make it even more work for me to try and get a  
review. In addition to going around trying to find someone to review  
my patch, now I have to periodically resubmit too. And this gets my  
patch reviewed faster how?

I do think there are real issues to be discussed here, and I think  
some of the ideas presented in other posts in this thread are worth  
taking a look at, but I just feel this approach is more a knee-jerk  
reaction rather than a real strategy to fix the review queue problem.

Regards,

Kevin

> We're down to 95 after today.  (We were at 144 two days ago!)
> curl -s "https://bugs.webkit.org/request.cgi" | grep PDT | wc -l
> 95
>
> -eric
>
> On Fri, May 22, 2009 at 2:38 PM, Maciej Stachowiak <mjs at apple.com>  
> wrote:
>>
>> On May 21, 2009, at 9:36 PM, Geoffrey Garen wrote:
>>
>> Our current defacto policy requires involvement on both sides.
>> Submitters need to be involved in finding people to review their
>> patches.  Posting patches to the review queue does not automatically
>> get you a review, except occasionally by Darin Adler or myself.
>>
>> If a bug totally stalls, and is sitting in the review queue untouched
>> I view that as the responsible reviewers' implicit rejection of the
>> patch.  I, as a responsible reviewer, am simply making explicit that
>> implicit rejection.  Personally, I'd rather get an r- on my patches
>> than have them sit ignored for multiple weeks at a time.
>>
>> Let's examine these statements in a broader light, substituting "bug
>> database" for "review queue" and "bug" for "patch":
>> Our current defacto policy requires involvement on both sides.  
>> Submitters
>> need to be involved in finding people to fix their bugs. Filing  
>> bugs to the
>> bug database does not automatically get you a fix, except  
>> occasionally by
>> Darin Adler or myself.
>> If a bug totally stalls, and is sitting in the bug database  
>> untouched, I
>> view that as the responsible reviewers' implicit rejection of the  
>> bug. I, as
>> a responsible reviewer, am simply making explicit that implicit  
>> rejection.
>> Personally, I'd rather get a "closed" on my bugs than have them sit  
>> ignored
>> for multiple weeks at a time.
>>
>> So, Eric, should we close all bugs that are older than 2 weeks?
>>
>> I thought of the same analogy, and for this reason I disagree with  
>> Eric's
>> proposed change. Marking patches r- without review feedback is  
>> impolite to
>> the patch submitter, and loses valuable information.
>> Regards,
>> Maciej
>>
> _______________________________________________
> webkit-dev mailing list
> webkit-dev at lists.webkit.org
> http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev