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 need

Re: [webkit-dev] review queue crazy idea

2010-07-24 Thread Ryosuke Niwa
On Sat, Jul 24, 2010 at 4:09 PM, Maciej Stachowiak m...@apple.com wrote: I think the main problems with http://trac.webkit.org/wiki/WebKit%20Team 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

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,

Re: [webkit-dev] review queue crazy idea

2010-07-23 Thread Alex Milowski
On Fri, Jul 23, 2010 at 12:51 PM, Eric Seidel e...@webkit.org 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

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

Re: [webkit-dev] review queue crazy idea

2010-07-23 Thread Alex Milowski
On Fri, Jul 23, 2010 at 1:11 PM, Eric Seidel e...@webkit.org 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

Re: [webkit-dev] review queue crazy idea

2010-07-23 Thread Alex Milowski
On Fri, Jul 23, 2010 at 1:15 PM, Alex Milowski a...@milowski.org wrote: On Fri, Jul 23, 2010 at 1:11 PM, Eric Seidel e...@webkit.org 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.

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

Re: [webkit-dev] review queue crazy idea

2010-07-22 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 just to

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-22 Thread Alex Milowski
On Thu, Jul 22, 2010 at 8:29 AM, Maciej Stachowiak m...@apple.com 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

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

[webkit-dev] review queue crazy idea

2010-07-21 Thread Ojan Vafai
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 bugs that haven't been touched in over a month. For people

Re: [webkit-dev] review queue crazy idea

2010-07-21 Thread Peter Kasting
On Wed, Jul 21, 2010 at 2:40 PM, Ojan Vafai o...@chromium.org 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.

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 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

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

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 Ryosuke Niwa
On Wed, Jul 21, 2010 at 2:40 PM, Ojan Vafai o...@chromium.org 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

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