[webkit-dev] Patch process - let's make it better

Maciej Stachowiak mjs at apple.com
Wed Jul 15 15:59:22 PDT 2009


I'll try to get bugs filed for any part of the plan that isn't done by  
tomorrow, and I'll tag them with a keyword so we can track progress.

On Jul 15, 2009, at 1:14 PM, Adam Treat wrote:

> On Friday 10 July 2009 06:55:59 pm Maciej Stachowiak wrote:
>> * Phase 1 *
>>
>> A) Make it really easy to submit a patch. Eric's bugzilla-tool is
>> close to being there. I'm going to help him refine this tool to the
>> point that submitting a patch has only one step - a single command
>> will make the patch, file a bug if needed, attach the patch to the
>> bug, and flag it for review.
>>
>> B) Make it really easy to commit a patch. Again, I think bugzilla- 
>> tool
>> is the right path to achieving this.
>>
>> C) Improve the way we get attention from reviewers. I think we should
>> do three things here:
>>     C.1) Split review queues, based on our emerging [Bracket]
>> convention for patches needing specialized review. If we find more
>> areas of specialty besides ports, we can add new [Bracket] tags.
>>     C.2) Improve the quality of emails sent
>>     C.3) Highlight in a special way patches that have been waiting
>> for review longer that some minimum period (a week?). These could be
>> highlighted visually in the review queue, and maybe would send extra
>> review request emails automatically.
>
> Speaking of your action plan... I've had a look at C.3 and was going  
> to try to
> modify the bugzilla tools to highlight as you suggested.  But after  
> looking at
> the review queue over the last few days I'm not sure it is necessary  
> or that
> it would help to fix the problem.
>
> http://www.webkit.org/pending-review
>
> The page above already lists the review queue sorted by date.  Over  
> the last
> few days the queue has numbered in the 100's.  A significant  
> fraction (50%+)
> are older than a week.  I could highlight them, but if it is already  
> ordered
> by date and the highlight will encompass more than half the queue...  
> what's
> the point?

I tend to use this URL to scan the review queue instead:

https://bugs.webkit.org/buglist.cgi?field0-0-0=flagtypes.name&type0-0-0=equals&value0-0-0=review%3F

I have  few problems with the official review queue page, and maybe  
what we need to do is come up with one version that has the benefits  
of both. Here are the things I don't like about the pending-review page:

1) Uses two lines per bug plus a table border, so you can see fewer  
items at once. I can see 13 in a max-height browser window on my  
MacBook Pro, compared to 25 using my preferred query.

2) It splits out bugs with a specific requestee into separate lists,  
this is I think harmful to prompt review because usually those bugs  
don't really need review from one specific person.

3) Lists multiple patches on a single bug separately. I don't like  
this, usually such patches should be reviewed together, and it  
inflates the list.

4) No summary count.

Here are things I like about it, compared to a vanilla query:

1) Skips most of the irrelevant fields, like priority, assignee,  
status, etc.

2) Sorts by date of request.

I would love to have a single view that combines the best of both ways  
of viewing the review queue.


That being said, I think highlighting old bugs is good to do right  
now. Even though today, 50% are older than a week, we don't want that  
to be the norm. By marking them in a clear way (and maybe further  
highlighting patches older than a month), we can help get to a point  
where this is the exception, not the norm.

Regards,
Maciej



More information about the webkit-dev mailing list