[webkit-dev] Review Queue

[Eric Seidel]

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

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
>