This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --TrLXbbjhE3oQVQMOFxEnvJ7bNR05SlUqO Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On Wed 12 Mar 2014 08:16:02 AM CET, Itamar Heim wrote:
On 03/11/2014 10:08 PM, Maor Lipchuk wrote:
On 03/11/2014 05:20 PM, Itamar Heim wrote:
On 03/11/2014 05:14 PM, Eyal Edri wrote:
----- Original Message -----
From: "Itamar Heim" <iheim@redhat.com> To: "Eyal Edri" <eedri@redhat.com>, "Tomasz Ko=C5=82ek" <tomasz-kolek@o2.pl>, users@ovirt.org, "infra" <infra@ovirt.org> Sent: Tuesday, March 11, 2014 5:10:54 PM Subject: Re: [Users] [GSOC][Gerrit] add potential reviewers - quest=
ions
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=C5=82ek 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 fl=
ags
>> should >> allow to add potential reviewers in gerrit. >> So: >> Let's assume that we've got special flags for this operations. W= hat'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 reviewer= s to > a patch according to the code sent? so in fact you'll have a pla= ce > somewhere for mapping code & specific developers?
I really like this idea. Gerrit currently requires new users to kn= ow who to add as reviewers, IMHO impeding new contributors.
One relative simple solution would be to look at who recently touc= hed 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 =3D set()
for modified_file in commit.files: reviewers +=3D set(commit.author for commit in git.log(modified_file))
return reviewers
This gives a system that those who touche a file, become the maint= ainer for that file. A more complex solution could improve on that and l= imit 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 =3D collections.Counter() # New in python 2.7
for modified_file in commit.files: reviewers +=3D 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/formattin= g 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? _______________________________________________ Users mailing list Users@ovirt.org http://lists.ovirt.org/mailman/listinfo/users
I think if we do this, we want to make sure we cover per file who i= s required to +2 it before we consider it acked.
won't it require maintaining static lists of people per file/path/project?
yes, but considering our project layout, i don't see an alternative. (some of the layout could be improved to be path based, rather than f= ile based) I think it could be done automatically by analysing the file and see w= ho mostly changed it recently, since the "owner" of the file might be dynamic, who ever changed most of it few days ago might be more famili= ar with it today
IMO the algorithm of adding the reviewers should be flexible. For example, using a folder which will contain files, where each file implement an algorithm to add the reviewers.
for instance we can have two files: 1. Add a reviewers by blame - the contributor which changed recently t= he code lines 2. Add a reviewers by file - the contributor who changed most of the file recently.
Each file will implement the functional operation and will output the reviewers emails.
The user can then add a new algorithm or change it to be more specific=
to its project. for example the user can add also the maintainers which acked the patc= h that was blamed.
_______________________________________________ Users mailing list Users@ovirt.org http://lists.ovirt.org/mailman/listinfo/users
this shouldn't be automatic. we need to clearly define ownership. we ca= n't do this per repo for the engine/vdsm. we can do this per repo for the othe= r repo's probably (though solving the folder/file approach would cover th= e simpler repos as a private case).
yes, it will require some work, maybe some moving around of files to ma= ke this easier by folders (topics) which should be relevant anyway.
I think it would easier to maintain if we just have one file at the=20 root, instead of having the ownership information distributed=20 throughout the files/directories. That way you'll know where to look at=20 to check/modify the ownership as opposed to having to walk all the=20 files and upper directories. Also, adding all that ownership logic to gerrit might not be easy, as=20 it's not meant to go checking the source of the repositories looking=20 for configuration. We might want to take a look (again) to zuul, the=20 tool that openstack uses as gateway to trigger jenkins jobs and merge=20 patches.
_______________________________________________ Infra mailing list Infra@ovirt.org http://lists.ovirt.org/mailman/listinfo/infra
-- David Caro Red Hat S.L. Continuous Integration Engineer - EMEA ENG Virtualization R&D Email: dcaro@redhat.com Web: www.redhat.com RHT Global #: 82-62605 --TrLXbbjhE3oQVQMOFxEnvJ7bNR05SlUqO Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQEcBAEBAgAGBQJTICJhAAoJEEBxx+HSYmnD5ccH+wbSsPmCAiK9sq8XPXvgaeMb 8x62a2DALqe5q8J4tGNQx/EtHw/GKFJT21jBiNHP5zZWbTSKzHcri2qlMPBvgsGY wN+A7QMOE9OhV1/FvkOoZjDRp2LFZ7GtAPkrJEljDVtCqOtyvkyekFeReMhraToW Gppx7/d3Z73tr4gh9r0J8+2r3/HHS4bZn71mWSGyogmMQpRUEPWfzZIuwKilmb3x b+RYxO72LB9JLQ3TKm1IZxHwyw93FRChWwkMGXYHrgNQ5yQKoltFuvz/Qa2yfVVh Qy32F1mxsAv9E0z0A0LuTHXJpvObf1PmMbvoICAc7ETxNOHtqGDLoTPNYPIyhl8= =qGqi -----END PGP SIGNATURE----- --TrLXbbjhE3oQVQMOFxEnvJ7bNR05SlUqO--