|Summary:||webkit-patch suggest-reviewers should limit itself to 5 reviewers|
|Product:||WebKit||Reporter:||Eric Seidel (no email) <eric>|
|Component:||Tools / Tests||Assignee:||Zan Dobersek <zan>|
|Severity:||Normal||CC:||abarth, dpranke, webkit.review.bot, zan|
|Version:||528+ (Nightly build)|
|Bug Depends on:|
Description Eric Seidel (no email) 2013-01-22 02:00:00 PST
webkit-patch suggest-reviewers should limit itself to 5 reviewers Right now it will CC an unbounded number, which is bad. Those should also be the 5 most relevant reviewers, ideally. Easy to calculate based on number of annotated lines in the diff attributed to each reviewer.
Comment 1 Zan Dobersek 2013-01-23 06:37:55 PST
(In reply to comment #0) > Those should also be the 5 most relevant reviewers, ideally. Easy to calculate based on number of annotated lines in the diff attributed to each reviewer. How about deciding about relevant reviewers by picking five who most recently reviewed patches around the code? This kind of guarantees that the offered reviewers is up-to-date with the current status of the code that's changed. I think that's also easier to implement than sorting by the sum of reviewed lines.
Comment 2 Zan Dobersek 2013-01-23 07:42:37 PST
Created attachment 184231 [details] Preliminary patch
Comment 3 Zan Dobersek 2013-01-23 07:46:55 PST
(In reply to comment #2) > Created an attachment (id=184231) [details] > Preliminary patch A WIP patch that outlines the 'five-latest-reviewers' approach. No unit test breakage. I believe this keeps the current behavior though it's probably not in line with what bug #107525 would like to accomplish.
Comment 4 Eric Seidel (no email) 2013-01-23 10:59:13 PST
Comment on attachment 184231 [details] Preliminary patch View in context: https://bugs.webkit.org/attachment.cgi?id=184231&action=review > Tools/Scripts/webkitpy/common/checkout/checkout.py:146 > + reviewers =  > + for info in commit_infos: > + if info.reviewer() and info.reviewer() not in reviewers: > + reviewers.append(info.reviewer()) > + if info.author() and info.author().can_review and info.author() not in reviewers: > + reviewers.append(info.author()) > + return reviewers It seems all you did here was remove the list comprehensions?
Comment 5 Zan Dobersek 2013-01-23 12:01:10 PST
Comment on attachment 184231 [details] Preliminary patch View in context: https://bugs.webkit.org/attachment.cgi?id=184231&action=review >> Tools/Scripts/webkitpy/common/checkout/checkout.py:146 >> + return reviewers > > It seems all you did here was remove the list comprehensions? This ugly piece of code actually just tries to use a list as an ordered set. The returned list contains all the reviewers connected to the changed files but lists them in order of their last activity, from most to least recent.
Comment 7 Eric Seidel (no email) 2013-02-15 12:38:37 PST
Comment on attachment 184447 [details] Patch OK.