Re: [Users] [GSOC][Gerrit] add potential reviewers - questions

On Tue 11 Mar 2014 04:14:26 PM CET, Eyal Edri wrote: > > > ----- Original Message ----- >> From: "Itamar Heim" <iheim(a)redhat.com&gt; >> To: "Eyal Edri" <eedri(a)redhat.com&gt;, "Tomasz Kołek" <tomasz-kolek(a)o2.pl&gt;, users(a)ovirt.org, "infra" <infra(a)ovirt.org&gt; >> Sent: Tuesday, March 11, 2014 5:10:54 PM >> Subject: Re: [Users] [GSOC][Gerrit] add potential reviewers - questions >> >> On 03/11/2014 05:06 PM, Ewoud Kohl van Wijngaarden wrote: >>> On Tue, Mar 11, 2014 at 10:37:22AM -0400, Eyal Edri wrote: >>>>> Tomasz Kołek wrote: >>>>> >>>>> I've got a few questions about project description. >>>>> Please tell me if my problem's understanding is good or not. >>>>> We need to add a few flags/methods to git review module. This flags >>>>> should >>>>> allow to add potential reviewers in gerrit. >>>>> So: >>>>> Let's assume that we've got special flags for this operations. What's >>>>> next? >>>>> 1. In gerrit system we need to add special place for potential reviewers? >>>>> 2. Potential reviewers should agree that they want to review? >>>>> 3. We can have more than one accepted reviewer? >>>> >>>> I'm not sure i understood exactly what you mean by 'potential >>>> reviewers'. do want gerrit (hook?) to automatically add reviewers to >>>> a patch according to the code sent? so in fact you'll have a place >>>> somewhere for mapping code & specific developers? >>> >>> I really like this idea. Gerrit currently requires new users to know who >>> to add as reviewers, IMHO impeding new contributors. >>> >>> One relative simple solution would be to look at who recently touched >>> the files that are being modified and add them as reviewers. This can be >>> done by looking at the git log for a file. Some pseudo python code >>> solution: >>> >>> reviewers = set() >>> >>> for modified_file in commit.files: >>> reviewers += set(commit.author for commit in git.log(modified_file)) >>> >>> return reviewers >>> >>> This gives a system that those who touche a file, become the maintainer >>> for that file. A more complex solution could improve on that and limit >>> the reviewers added per patch. One can think of limiting to only >>> contributions in the last X months, weigh contributions so common >>> committers are prefered. It could also combine several methods. >>> >>> For example to limit to the 5 authors who touched the most files: >>> >>> reviewers = collections.Counter() # New in python 2.7 >>> >>> for modified_file in commit.files: >>> reviewers += collections.Counter(commit.author for commit in >>> git.log(modified_file)) >>> >>> return [author for author, count in reviewers.most_common(5)] >>> >>> Since Counter also accepts a dictionary, one could also weigh the >>> touched lines per file. Downside there is big whitespace/formatting >>> patches can skew the line count. >>> >>> In short, I think an entire thesis could be written on the optimal way >>> to determine reviewers but a simple algorithm could do to show the >>> method works. >>> >>> Does this help? >> >> I think if we do this, we want to make sure we cover per file who is >> required to +2 it before we consider it acked. >> > > won't it require maintaining static lists of people per file/path/project?

About dynamically adding reviewers, it should be fairly easy to do using a small gerrit hook to run on patch-submitted. But gerrit will fail if the user is not already created in gerrit so not really allowing non-registered users to get in. But I'm not sure if it's 'polite' to add someone as a reviewer if he did not previously agreed to it, specially when emails will be sent to him.