12:01 p.m.
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(a)redhat.com>
>>>> To: "Eyal Edri" <eedri(a)redhat.com>, "Tomasz
Ko=C5=82ek"
>>>> <tomasz-kolek(a)o2.pl>, users(a)ovirt.org, "infra"
<infra(a)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(a)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(a)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(a)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(a)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--