Re: [webkit-dev] review queue crazy idea

2010-07-24 Thread Ryosuke Niwa
On Sat, Jul 24, 2010 at 4:09 PM, Maciej Stachowiak wrote: > I think the main problems with > are that (a) people don't know to look there; and (b) people don't know or > don't bother to update it. > I totally agree. I'll also add that the information

Re: [webkit-dev] review queue crazy idea

2010-07-24 Thread Maciej Stachowiak
On Jul 23, 2010, at 1:10 PM, Dirk Pranke wrote: > I have been thinking along these lines as well. I'm not sure how > relevant touching existing lines of code is versus just other people > who have hacked on the file at all or who have hacked on other files > in the same directory (i.e., you'd nee

Re: [webkit-dev] review queue crazy idea

2010-07-23 Thread Dirk Pranke
I have been thinking along these lines as well. I'm not sure how relevant touching existing lines of code is versus just other people who have hacked on the file at all or who have hacked on other files in the same directory (i.e., you'd need to address new code and new files, too). I think some em

Re: [webkit-dev] review queue crazy idea

2010-07-23 Thread Alex Milowski
On Fri, Jul 23, 2010 at 1:15 PM, Alex Milowski wrote: > On Fri, Jul 23, 2010 at 1:11 PM, Eric Seidel wrote: >> Given a patch file, you have its line number ranges. >> >> Given a git checkout, you can very quickly find who has made changes >> to what lines in that file. >> >> You then can have a b

Re: [webkit-dev] review queue crazy idea

2010-07-23 Thread Alex Milowski
On Fri, Jul 23, 2010 at 1:11 PM, Eric Seidel wrote: > Given a patch file, you have its line number ranges. > > Given a git checkout, you can very quickly find who has made changes > to what lines in that file. > > You then can have a bot post to the bug, saying that 10 people have > touched the li

Re: [webkit-dev] review queue crazy idea

2010-07-23 Thread Eric Seidel
Given a patch file, you have its line number ranges. Given a git checkout, you can very quickly find who has made changes to what lines in that file. You then can have a bot post to the bug, saying that 10 people have touched the lines you're touching in your patch. 3 of them are active reviewer

Re: [webkit-dev] review queue crazy idea

2010-07-23 Thread Alex Milowski
On Fri, Jul 23, 2010 at 12:51 PM, Eric Seidel wrote: > I've never really liked trac.webkit.org/wiki/WebKit%20Team.  Its > always seemed more of place to brag about webkit involvement, than a > useful reference.  I think we could build a much better "who should I > ask to review this" tool based on

Re: [webkit-dev] review queue crazy idea

2010-07-23 Thread Eric Seidel
I've never really liked trac.webkit.org/wiki/WebKit%20Team. Its always seemed more of place to brag about webkit involvement, than a useful reference. I think we could build a much better "who should I ask to review this" tool based on SVN information. -eric On Fri, Jul 23, 2010 at 12:15 AM, Da

Re: [webkit-dev] review queue crazy idea

2010-07-22 Thread David Kilzer
We should also publicize/update these existing resources to help patch authors find reviewers for their patches: http://trac.webkit.org/wiki/CodeReview http://trac.webkit.org/wiki/WebKit%20Team I think the most effective approach is when patch authors proactively seek out reviewers. We're all

Re: [webkit-dev] review queue crazy idea

2010-07-22 Thread Ojan Vafai
On Wed, Jul 21, 2010 at 4:47 PM, Eric Seidel wrote: > Patches sit in the queue for numerous reasons. Some of us used to > scan the queue every day. But I've stopped doing that. Now I scan it > more like once a week or two. > > There is no good way to find "which patches might I have a chance o

Re: [webkit-dev] review queue crazy idea

2010-07-22 Thread Alex Milowski
On Thu, Jul 22, 2010 at 8:29 AM, Maciej Stachowiak wrote: > > > I think a better UI for reviews, plus some better attempts at active > notification of potential reviewers, could go a long way. I'm a strong > believer in trying nudges and positive incentives before implementing harsher > policie

Re: [webkit-dev] review queue crazy idea

2010-07-22 Thread Maciej Stachowiak
On Jul 21, 2010, at 3:41 PM, Eric Seidel wrote: > Wow. I really like this idea of helping contributors better > understand what's going wrong. > > But, I think that even better would be to build a better front-end for > reviews. Or a bot which knew how to suggest reviewers (based on > annotate

Re: [webkit-dev] review queue crazy idea

2010-07-21 Thread Zoltan Herczeg
> Patches sit in the queue for numerous reasons. Some of us used to > scan the queue every day. But I've stopped doing that. Now I scan it > more like once a week or two. > > There is no good way to find "which patches might I have a chance of > reviewing", so you end up spending 30 minutes jus

Re: [webkit-dev] review queue crazy idea

2010-07-21 Thread James Robinson
I've had patches sit in the review queue for >4 weeks then receive a positive review and land without much incident. Some patches are difficult to review or have a limited number of potential reviewers. I would have really appreciated a reminder email about that patch in particular (I honestly ha

Re: [webkit-dev] review queue crazy idea

2010-07-21 Thread Ryosuke Niwa
On Wed, Jul 21, 2010 at 2:40 PM, Ojan Vafai wrote: > There are currently 38 (of 171 total) patches in the review queue where > the bugs have not been modified in over 1 month old. I propose we have a bot > that educates people about writing easy to review patches and auto-rejects > any patches in

Re: [webkit-dev] review queue crazy idea

2010-07-21 Thread Eric Seidel
Patches sit in the queue for numerous reasons. Some of us used to scan the queue every day. But I've stopped doing that. Now I scan it more like once a week or two. There is no good way to find "which patches might I have a chance of reviewing", so you end up spending 30 minutes just to find a

Re: [webkit-dev] review queue crazy idea

2010-07-21 Thread Zoltan Horvath
Hey, I just don't understand how can the patches can sit in bugzilla unreviewed for weeks? There are 76 reviewers in the trac's team list. I think the reviewers have to do what they have assumed... Reviewing patches! I agree with Maciej automatic rejection is unfriendly. (Of course we can reje

Re: [webkit-dev] review queue crazy idea

2010-07-21 Thread Eric Seidel
Wow. I really like this idea of helping contributors better understand what's going wrong. But, I think that even better would be to build a better front-end for reviews. Or a bot which knew how to suggest reviewers (based on annotate information from lines changed). I encourage you to write an

Re: [webkit-dev] review queue crazy idea

2010-07-21 Thread Maciej Stachowiak
On Jul 21, 2010, at 2:40 PM, Ojan Vafai wrote: > There are currently 38 (of 171 total) patches in the review queue where the > bugs have not been modified in over 1 month old. I propose we have a bot that > educates people about writing easy to review patches and auto-rejects any > patches in

Re: [webkit-dev] review queue crazy idea

2010-07-21 Thread Peter Kasting
On Wed, Jul 21, 2010 at 2:40 PM, Ojan Vafai wrote: > Here are my initial thoughts on what a review bot would do. > > *After a patch turns a week old, send the following email:* > Patch 12345 of bug 6789 is a week old. It may just be because no reviewer > has found time to review it. But there may

Re: [webkit-dev] Review Queue

2009-09-02 Thread Eric Seidel
> > > Honestly, I was probably a bit aggressive in r-ing your patches. > Hopefully you won't take that personally. My hope was to get the ball > rolling again because the work on the WINCE port seemed to have > stalled out. Having a bunch of r? patches languishing in the review > queue isn't real

Re: [webkit-dev] Review Queue

2009-09-02 Thread Adam Barth
On Wed, Sep 2, 2009 at 9:45 AM, Yong Li wrote: > I'm a very first user of check-webkit-style (was called cpplint when I used > it), and reported some bugs of it. The current indentation issue of some > construtor inialization list were actually introduced by following the > suggestion of old versio

Re: [webkit-dev] Review Queue

2009-09-02 Thread Yong Li
;Eric Seidel" ; "WebKit Development" Sent: Wednesday, September 02, 2009 9:36 AM Subject: Re: [webkit-dev] Review Queue Hi Yong, Sorry for r-ing a bunch of your patches last night. I tried to give you constructive comments. As you might be aware, we have a style linter now: ./WebK

Re: [webkit-dev] Review Queue

2009-09-02 Thread Adam Barth
Hi Yong, Sorry for r-ing a bunch of your patches last night. I tried to give you constructive comments. As you might be aware, we have a style linter now: ./WebKitTools/Scripts/check-webkit-style It might help us skip past some of the trivial review comments (like improper indenting of initial

Re: [webkit-dev] Review Queue

2009-09-02 Thread Yong Li
Thanks a lot for spending time on reviewing. As a developer, I hope I can have more constructive comments for r-. I understand it's a hard work and truly appreciate all of you who spend time on it. -Yong - Original Message - From: Eric Seidel To: WebKit Development Sent: Wedne

Re: [webkit-dev] Review queue needs love

2009-06-18 Thread Adam Treat
On Thursday 18 June 2009 05:39:04 pm Oliver Hunt wrote: > I reviewed quite a few last night as well. At the moment there appear > to be a large number of chromium, gtk, and qt specific patches up for > review -- it would be great if reviewers for those ports went through > them all :D I went thro

Re: [webkit-dev] Review queue needs love

2009-06-18 Thread Oliver Hunt
I reviewed quite a few last night as well. At the moment there appear to be a large number of chromium, gtk, and qt specific patches up for review -- it would be great if reviewers for those ports went through them all :D --Oliver On Jun 18, 2009, at 2:27 PM, Adam Barth wrote: I'll do m

Re: [webkit-dev] Review queue needs love

2009-06-18 Thread Adam Barth
I'll do my best tonight. Adam On Thu, Jun 18, 2009 at 9:26 AM, Maciej Stachowiak wrote: > > 54 bugs with patches to review: > > https://bugs.webkit.org/buglist.cgi?field0-0-0=flagtypes.name&type0-0-0=equals&value0-0-0=review%3F > > I reviewed a few just now, I'll try to do more today. Anyone els

Re: [webkit-dev] Review Queue

2009-06-05 Thread Eric Seidel
Ok, I agree that a sectioned queue would help. There is no need to see the Gtk patches in the list: https://bugs.webkit.org/buglist.cgi?field0-0-0=flagtypes.name&type0-0-0=equals&value0-0-0=review%3F (which is back up to 40, btw!) But we should be able to keep "core" (nearly) empty if it were kep

Re: [webkit-dev] Review Queue

2009-05-22 Thread Eric Seidel
We're down to 30 bugs: https://bugs.webkit.org/buglist.cgi?field0-0-0=flagtypes.name&type0-0-0=equals&value0-0-0=review%3F With 36 patches total: curl -s "https://bugs.webkit.org/request.cgi"; | grep PDT | wc -l 15 of those are Gtk-only bugs. Need some help from the Gtk reviewers on this one. T

Re: [webkit-dev] Review Queue

2009-05-22 Thread Eric Seidel
Reviewed more before bed. We're down to: https://bugs.webkit.org/buglist.cgi?field0-0-0=flagtypes.name&type0-0-0=equals&value0-0-0=review%3F 50 and curl -s "https://bugs.webkit.org/request.cgi"; | grep PDT | wc -l 61 respectively. Still awful, but much much better than yesterday. -eric On F

Re: [webkit-dev] Review Queue

2009-05-22 Thread Gustavo Noronha
On Thu, 2009-05-21 at 21:41 -0700, Maciej Stachowiak wrote: > I discussed the review backlog with Mark Rowe earlier, and we came up > with another idea that may help. This would be to categorize the > review queues. Perhaps we could get bugzilla to show a separate review > queue per component. So

Re: [webkit-dev] Review Queue

2009-05-22 Thread Nikolas Zimmermann
Am 22.05.2009 um 06:41 schrieb Maciej Stachowiak: On May 21, 2009, at 9:18 PM, Ojan Vafai wrote: It makes no sense to me to r- a patch because reviewers don't have time to review it. It put incentive in the wrong place. There are other solutions to this problem that put incentive in the r

Re: [webkit-dev] Review Queue

2009-05-22 Thread Maciej Stachowiak
On May 22, 2009, at 2:19 AM, Eric Seidel wrote: Update: We're down to 74 patches now. Thanks especially to Maciej for all his reviewing this evening: curl -s "https://bugs.webkit.org/request.cgi"; | grep PDT | wc -l] 74 Still a long way to go. FWIW I prefer this query, which counts by bug

Re: [webkit-dev] Review Queue

2009-05-22 Thread Eric Seidel
Update: We're down to 74 patches now. Thanks especially to Maciej for all his reviewing this evening: curl -s "https://bugs.webkit.org/request.cgi"; | grep PDT | wc -l] 74 Still a long way to go. -eric On Fri, May 22, 2009 at 12:27 PM, Eric Seidel wrote: > Our review process seems to be fail

Re: [webkit-dev] Review Queue

2009-05-21 Thread Zoltan Herczeg
Hi, > 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

Re: [webkit-dev] Review Queue

2009-05-21 Thread 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

Re: [webkit-dev] Review Queue

2009-05-21 Thread Peter Kasting
On Thu, May 21, 2009 at 10:08 PM, Maciej Stachowiak wrote: > If you want to r- a patch for a reason, such as being too big, or having > feedback already that hasn't been addressed, that's fine. But I think it > would be a bad idea to reject patches just because they haven't been > reviewed for to

Re: [webkit-dev] Review Queue

2009-05-21 Thread Maciej Stachowiak
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 closi

Re: [webkit-dev] Review Queue

2009-05-21 Thread 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 th

Re: [webkit-dev] Review Queue

2009-05-21 Thread Maciej Stachowiak
On May 21, 2009, at 9:10 PM, Peter Kasting wrote: On Thu, May 21, 2009 at 8:52 PM, Eric Seidel 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 automatic

Re: [webkit-dev] Review Queue

2009-05-21 Thread Maciej Stachowiak
On May 21, 2009, at 9:18 PM, Ojan Vafai wrote: It makes no sense to me to r- a patch because reviewers don't have time to review it. It put incentive in the wrong place. There are other solutions to this problem that put incentive in the right place (i.e. with reviewers). I don't necessari

Re: [webkit-dev] Review Queue

2009-05-21 Thread Maciej Stachowiak
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

Re: [webkit-dev] Review Queue

2009-05-21 Thread Geoffrey Garen
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 sitt

Re: [webkit-dev] Review Queue

2009-05-21 Thread Ojan Vafai
It makes no sense to me to r- a patch because reviewers don't have time to review it. It put incentive in the wrong place. There are other solutions to this problem that put incentive in the right place (i.e. with reviewers). I don't necessarily think these are good ideas, but I'll throw them out t

Re: [webkit-dev] Review Queue

2009-05-21 Thread Peter Kasting
On Thu, May 21, 2009 at 8:52 PM, Eric Seidel 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

Re: [webkit-dev] Review Queue

2009-05-21 Thread Eric Seidel
Sorry I missed on on IRC, I would have been happy to chat further. 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 occasion

Re: [webkit-dev] Review Queue

2009-05-21 Thread Maciej Stachowiak
On May 21, 2009, at 7:57 PM, Eric Seidel wrote: I think it's better to get things out of the queue then to leave them rot. But it's not the patch submitter's fault if the reviewers have been delinquent in review. And making them resubmit the same patch after a blanket r- is useless busyw

Re: [webkit-dev] Review Queue

2009-05-21 Thread Eric Seidel
I think it's better to get things out of the queue then to leave them rot. Your review are most welcome. :) -eric On Fri, May 22, 2009 at 12:48 PM, Maciej Stachowiak wrote: > > On May 21, 2009, at 7:27 PM, Eric Seidel wrote: > >> Our review process seems to be failing.  As a reviewer, let me ex

Re: [webkit-dev] Review Queue

2009-05-21 Thread Maciej Stachowiak
On May 21, 2009, at 7:27 PM, Eric Seidel wrote: Our review process seems to be failing. As a reviewer, let me extend my apologies to the WebKit community as I am part of this failure. We have over 100 patches in the review queue at the moment: http://webkit.org/pending-review I've started go