Bug 107528 – webkit-patch suggest-reviewers should limit itself to 5 reviewers

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Eric Seidel (no email)]

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.

[Zan Dobersek]

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

[Zan Dobersek]

Created attachment 184231 [details]
Preliminary patch

[Zan Dobersek]

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

[Eric Seidel (no email)]

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?

[Zan Dobersek]

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.

[Zan Dobersek]

Created attachment 184447 [details]
Patch

[Eric Seidel (no email)]

Comment on attachment 184447 [details]
Patch

OK.

[Zan Dobersek]

Committed r143033: <http://trac.webkit.org/changeset/143033>