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
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
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,
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
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
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
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.
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
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
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
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
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
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
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.
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
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
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
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
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
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
a hard work and
truly appreciate all of you who spend time on it.
-Yong
- Original Message -
From: Eric Seidel
To: WebKit Development
Sent: Wednesday, September 02, 2009 6:10 AM
Subject: [webkit-dev] Review Queue
Just your friendly reminder that the review-queue is out of control
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 really
We have close to 100 patches that need attention in the review queue.
http://www.webkit.org/pending-review
I count five or six for the Qt port. Eight for the GTK port. Several for the
Haiku port where a decision should probably be made. And several for the ARM
Jit work and the custom memory
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
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 through
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.nametype0-0-0=equalsvalue0-0-0=review%3F
(which is back up to 40, btw!)
But we should be able to keep core (nearly) empty if it were kept
We're down to 30 bugs:
https://bugs.webkit.org/buglist.cgi?field0-0-0=flagtypes.nametype0-0-0=equalsvalue0-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.
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
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
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
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 for
Reviewed more before bed. We're down to:
https://bugs.webkit.org/buglist.cgi?field0-0-0=flagtypes.nametype0-0-0=equalsvalue0-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 Fri,
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 going through the list and reviewing what patches I
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
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 m...@apple.com wrote:
On May 21, 2009, at 7:27 PM, Eric Seidel wrote:
Our review process seems to be failing. As a reviewer,
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
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
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
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
On May 21, 2009, at 9:10 PM, Peter Kasting wrote:
On Thu, May 21, 2009 at 8:52 PM, Eric Seidel e...@webkit.org 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
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
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
On Thu, May 21, 2009 at 10:08 PM, Maciej Stachowiak m...@apple.com 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
43 matches
Mail list logo
