[Users] [GSOC][Gerrit] add potential reviewers - questions
This is a multipart message in MIME format. ------=_NextPart_000_0007_01CF3CA3.354BF560 Content-Type: text/plain; charset="iso-8859-2" Content-Transfer-Encoding: 7bit Hi I'd like to contribute (within GSOC) in idea: Gerrit add potential reviewers. Maybe at first I'll introduce myself. I'm Tomek, student from Poland. I study Computer Science at University of Wroclaw. The next year will be last year of my first degree study, I hope. As a python programmer I'm working since one year at Nokia Solutions and Networks (don't worry I intend to change my job to another or to participation at GSOC). Every day at work and school I'm using version control system (Git and SVN). At work we were using to Gerrit as a review system but currently we're using JIRA to report review statuses. I love spend my free time in mountains (mainly polish - Tatras mountains). That's all about me. 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 know above questions might seem chaotic but I think answers allow me to better understanding Yours needments Best regards Tomasz Kolek ------=_NextPart_000_0007_01CF3CA3.354BF560 Content-Type: text/html; charset="iso-8859-2" Content-Transfer-Encoding: quoted-printable <html xmlns:v=3D"urn:schemas-microsoft-com:vml" = xmlns:o=3D"urn:schemas-microsoft-com:office:office" = xmlns:w=3D"urn:schemas-microsoft-com:office:word" = xmlns:m=3D"http://schemas.microsoft.com/office/2004/12/omml" = xmlns=3D"http://www.w3.org/TR/REC-html40"><head><meta = http-equiv=3DContent-Type content=3D"text/html; = charset=3Diso-8859-2"><meta name=3DGenerator content=3D"Microsoft Word = 14 (filtered medium)"><style><!-- /* Font Definitions */ @font-face {font-family:Calibri; panose-1:2 15 5 2 2 2 4 3 2 4;} /* Style Definitions */ p.MsoNormal, li.MsoNormal, div.MsoNormal {margin:0in; margin-bottom:.0001pt; font-size:11.0pt; font-family:"Calibri","sans-serif"; mso-fareast-language:EN-US;} a:link, span.MsoHyperlink {mso-style-priority:99; color:blue; text-decoration:underline;} a:visited, span.MsoHyperlinkFollowed {mso-style-priority:99; color:purple; text-decoration:underline;} span.Stylwiadomocie-mail17 {mso-style-type:personal-compose; font-family:"Calibri","sans-serif"; color:windowtext;} .MsoChpDefault {mso-style-type:export-only; font-family:"Calibri","sans-serif"; mso-fareast-language:EN-US;} @page WordSection1 {size:8.5in 11.0in; margin:70.85pt 70.85pt 70.85pt 70.85pt;} div.WordSection1 {page:WordSection1;} --></style><!--[if gte mso 9]><xml> <o:shapedefaults v:ext=3D"edit" spidmax=3D"1026" /> </xml><![endif]--><!--[if gte mso 9]><xml> <o:shapelayout v:ext=3D"edit"> <o:idmap v:ext=3D"edit" data=3D"1" /> </o:shapelayout></xml><![endif]--></head><body lang=3DPL link=3Dblue = vlink=3Dpurple><div class=3DWordSection1><p class=3DMsoNormal><span = lang=3DEN-US>Hi <o:p></o:p></span></p><p class=3DMsoNormal><span = lang=3DEN-US>I'd like to contribute (within GSOC) in idea: Gerrit add = potential reviewers.<br><br>Maybe at first I’ll introduce = myself.<br>I'm Tomek, student from Poland. I study Computer Science at = University of Wroclaw. The next year will be last year of my first = degree study, I hope.<br>As a python programmer I'm working since one = year at Nokia Solutions and Networks (don't worry I intend to change my = job to another or to participation at GSOC). Every day at work and = school I'm using version control system (Git and SVN). At work we were = using to Gerrit as a review system but currently we're using JIRA to = report review statuses.<br>I love spend my free time in mountains = (mainly polish - Tatras mountains).<br>That's all about me.<br><br>I've = got a few questions about project description.<br>Please tell me if my = problem's understanding is good or not.<br>We need to add a few = flags/methods to git review module. This flags should allow to add = potential reviewers in gerrit.<br>So:<br>Let's assume that we've got = special flags for this operations. What's next?<br>1. In gerrit system = we need to add special place for potential reviewers?<br>2. Potential = reviewers should agree that they want to review?<br>3. We can have more = than one accepted reviewer?<br><br>I know above questions might seem = chaotic but I think answers allow me to better understanding Yours = needments<br><br>Best regards<br>Tomasz = Kolek<o:p></o:p></span></p></div></body></html> ------=_NextPart_000_0007_01CF3CA3.354BF560--
----- Original Message -----
From: "Tomasz Kołek" <tomasz-kolek@o2.pl> To: users@ovirt.org Sent: Monday, March 10, 2014 9:56:28 PM Subject: [Users] [GSOC][Gerrit] add potential reviewers - questions
Hi
I'd like to contribute (within GSOC) in idea: Gerrit add potential reviewers.
Maybe at first I’ll introduce myself. I'm Tomek, student from Poland. I study Computer Science at University of Wroclaw. The next year will be last year of my first degree study, I hope. As a python programmer I'm working since one year at Nokia Solutions and Networks (don't worry I intend to change my job to another or to participation at GSOC). Every day at work and school I'm using version control system (Git and SVN). At work we were using to Gerrit as a review system but currently we're using JIRA to report review statuses. I love spend my free time in mountains (mainly polish - Tatras mountains). That's all about me.
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?
Hi Tomasz, 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? cause if you mean by adding reviewers to a patch, that's easily done by just clicking the '+' sign on each patch. those reviewers should have logged in at least once to gerrit.ovirt.org in order to be in the list of potential viewers, and they don't require any special permissions to review with +1/-1 any patch sent. you can add as many reviewers as you want to a patch. does that answer your question? Eyal.
I know above questions might seem chaotic but I think answers allow me to better understanding Yours needments
Best regards Tomasz Kolek
_______________________________________________ Users mailing list Users@ovirt.org http://lists.ovirt.org/mailman/listinfo/users
Eyal, I think that he's asking about [1]. As far as I understand, the idea is to automatically get the reviewers names, with commands like `git blame`, or from gerrit - get previous reviewers to the same file/module... Adding Maor, since he might have some additional info. [1] http://www.ovirt.org/Summer_of_Code#Idea:_Gerrit_add_potential_reviewers ----- Original Message -----
From: "Eyal Edri" <eedri@redhat.com> To: "Tomasz Kołek" <tomasz-kolek@o2.pl> Cc: users@ovirt.org, "infra" <infra@ovirt.org> Sent: Tuesday, March 11, 2014 4:37:22 PM Subject: Re: [Users] [GSOC][Gerrit] add potential reviewers - questions
----- Original Message -----
From: "Tomasz Kołek" <tomasz-kolek@o2.pl> To: users@ovirt.org Sent: Monday, March 10, 2014 9:56:28 PM Subject: [Users] [GSOC][Gerrit] add potential reviewers - questions
Hi
I'd like to contribute (within GSOC) in idea: Gerrit add potential reviewers.
Maybe at first I’ll introduce myself. I'm Tomek, student from Poland. I study Computer Science at University of Wroclaw. The next year will be last year of my first degree study, I hope. As a python programmer I'm working since one year at Nokia Solutions and Networks (don't worry I intend to change my job to another or to participation at GSOC). Every day at work and school I'm using version control system (Git and SVN). At work we were using to Gerrit as a review system but currently we're using JIRA to report review statuses. I love spend my free time in mountains (mainly polish - Tatras mountains). That's all about me.
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?
Hi Tomasz, 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?
cause if you mean by adding reviewers to a patch, that's easily done by just clicking the '+' sign on each patch. those reviewers should have logged in at least once to gerrit.ovirt.org in order to be in the list of potential viewers, and they don't require any special permissions to review with +1/-1 any patch sent. you can add as many reviewers as you want to a patch.
does that answer your question?
Eyal.
I know above questions might seem chaotic but I think answers allow me to better understanding Yours needments
Best regards Tomasz Kolek
_______________________________________________ Users mailing list Users@ovirt.org http://lists.ovirt.org/mailman/listinfo/users
_______________________________________________ Users mailing list Users@ovirt.org http://lists.ovirt.org/mailman/listinfo/users
Hi Tomasz, I'm very please to hear that you are interested in the GSOC project, see my answers inline. It's great to see that you know the material pretty well, I'm interested to hear some more ideas and feedbacks. I will start a thread soon (Maybe also schedule a call) once getting more feedbacks from other contributors, so we can brain storm some more and get practical with it. If you have any more questions, please don't hesitate to ask me. thanks, Maor On 03/11/2014 04:53 PM, Meital Bourvine wrote:
Eyal,
I think that he's asking about [1].
As far as I understand, the idea is to automatically get the reviewers names, with commands like `git blame`, or from gerrit - get previous reviewers to the same file/module...
Adding Maor, since he might have some additional info.
[1] http://www.ovirt.org/Summer_of_Code#Idea:_Gerrit_add_potential_reviewers
----- Original Message -----
From: "Eyal Edri" <eedri@redhat.com> To: "Tomasz Kołek" <tomasz-kolek@o2.pl> Cc: users@ovirt.org, "infra" <infra@ovirt.org> Sent: Tuesday, March 11, 2014 4:37:22 PM Subject: Re: [Users] [GSOC][Gerrit] add potential reviewers - questions
----- Original Message -----
From: "Tomasz Kołek" <tomasz-kolek@o2.pl> To: users@ovirt.org Sent: Monday, March 10, 2014 9:56:28 PM Subject: [Users] [GSOC][Gerrit] add potential reviewers - questions
Hi
I'd like to contribute (within GSOC) in idea: Gerrit add potential reviewers.
Maybe at first I’ll introduce myself. I'm Tomek, student from Poland. I study Computer Science at University of Wroclaw. The next year will be last year of my first degree study, I hope. As a python programmer I'm working since one year at Nokia Solutions and Networks (don't worry I intend to change my job to another or to participation at GSOC). Every day at work and school I'm using version control system (Git and SVN). At work we were using to Gerrit as a review system but currently we're using JIRA to report review statuses. I love spend my free time in mountains (mainly polish - Tatras mountains). That's all about me.
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? Adding a plugin to gerrit is an interesting idea although it will require the administrator to add it in the server, and I want that the operation will be more available to any submitter in any project. I think that we can address it in a later phase though. 2. Potential reviewers should agree that they want to review? no, since gerrit does not question the reviewer if he wants to be added or not I think we should keep this behaviour as well.
although as I see it, the operation of adding the reviewers, will not be completely automatic for the submitter, we should let the submitter of the patch to choose which reviewers he wants to add, very similar to the screen being used when doing a rebase in git. (see attached file add_reviewers.png)
3. We can have more than one accepted reviewer? Yes, that is correct.
Hi Tomasz, 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?
cause if you mean by adding reviewers to a patch, that's easily done by just clicking the '+' sign on each patch. those reviewers should have logged in at least once to gerrit.ovirt.org in order to be in the list of potential viewers, and they don't require any special permissions to review with +1/-1 any patch sent. you can add as many reviewers as you want to a patch.
does that answer your question?
Eyal.
I know above questions might seem chaotic but I think answers allow me to better understanding Yours needments
Best regards Tomasz Kolek
_______________________________________________ Users mailing list Users@ovirt.org http://lists.ovirt.org/mailman/listinfo/users
_______________________________________________ Users mailing list Users@ovirt.org http://lists.ovirt.org/mailman/listinfo/users
Hi, Thank You for the answers, these give me basic look at the problem. Is not a problem (from my point of view) to implement a few flags for the command, like: add reviewers --auto --max=5 --using_blame etc, depends only on our requirements. I'm almost sure that good way is automatic way as Eyal mentioned, I know it might make a little mess but will be easier for new contributor. Why almost? Because what with a situation where all of potential reviewers are not interested to review patch? Maybe we should try to find "the most reviewing" people, maybe it will be good way? BR, Tomek -----Original Message----- From: Maor Lipchuk [mailto:mlipchuk@redhat.com] Sent: Tuesday, March 11, 2014 8:40 PM To: Meital Bourvine; Eyal Edri Cc: Tomasz Kołek; users@ovirt.org; infra Subject: Re: [Users] [GSOC][Gerrit] add potential reviewers - questions Hi Tomasz, I'm very please to hear that you are interested in the GSOC project, see my answers inline. It's great to see that you know the material pretty well, I'm interested to hear some more ideas and feedbacks. I will start a thread soon (Maybe also schedule a call) once getting more feedbacks from other contributors, so we can brain storm some more and get practical with it. If you have any more questions, please don't hesitate to ask me. thanks, Maor On 03/11/2014 04:53 PM, Meital Bourvine wrote:
Eyal,
I think that he's asking about [1].
As far as I understand, the idea is to automatically get the reviewers names, with commands like `git blame`, or from gerrit - get previous reviewers to the same file/module...
Adding Maor, since he might have some additional info.
[1] http://www.ovirt.org/Summer_of_Code#Idea:_Gerrit_add_potential_reviewe rs
----- Original Message -----
From: "Eyal Edri" <eedri@redhat.com> To: "Tomasz Kołek" <tomasz-kolek@o2.pl> Cc: users@ovirt.org, "infra" <infra@ovirt.org> Sent: Tuesday, March 11, 2014 4:37:22 PM Subject: Re: [Users] [GSOC][Gerrit] add potential reviewers - questions
----- Original Message -----
From: "Tomasz Kołek" <tomasz-kolek@o2.pl> To: users@ovirt.org Sent: Monday, March 10, 2014 9:56:28 PM Subject: [Users] [GSOC][Gerrit] add potential reviewers - questions
Hi
I'd like to contribute (within GSOC) in idea: Gerrit add potential reviewers.
Maybe at first I’ll introduce myself. I'm Tomek, student from Poland. I study Computer Science at University of Wroclaw. The next year will be last year of my first degree study, I hope. As a python programmer I'm working since one year at Nokia Solutions and Networks (don't worry I intend to change my job to another or to participation at GSOC). Every day at work and school I'm using version control system (Git and SVN). At work we were using to Gerrit as a review system but currently we're using JIRA to report review statuses. I love spend my free time in mountains (mainly polish - Tatras mountains). That's all about me.
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? Adding a plugin to gerrit is an interesting idea although it will require the administrator to add it in the server, and I want that the operation will be more available to any submitter in any project. I think that we can address it in a later phase though. 2. Potential reviewers should agree that they want to review? no, since gerrit does not question the reviewer if he wants to be added or not I think we should keep this behaviour as well.
although as I see it, the operation of adding the reviewers, will not be completely automatic for the submitter, we should let the submitter of the patch to choose which reviewers he wants to add, very similar to the screen being used when doing a rebase in git. (see attached file add_reviewers.png)
3. We can have more than one accepted reviewer? Yes, that is correct.
Hi Tomasz, 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?
cause if you mean by adding reviewers to a patch, that's easily done by just clicking the '+' sign on each patch. those reviewers should have logged in at least once to gerrit.ovirt.org in order to be in the list of potential viewers, and they don't require any special permissions to review with +1/-1 any patch sent. you can add as many reviewers as you want to a patch.
does that answer your question?
Eyal.
I know above questions might seem chaotic but I think answers allow me to better understanding Yours needments
Best regards Tomasz Kolek
_______________________________________________ Users mailing list Users@ovirt.org http://lists.ovirt.org/mailman/listinfo/users
_______________________________________________ Users mailing list Users@ovirt.org http://lists.ovirt.org/mailman/listinfo/users
On 03/11/2014 11:17 PM, Tomasz Kołek wrote:
Hi, Thank You for the answers, these give me basic look at the problem.
Is not a problem (from my point of view) to implement a few flags for the command, like: add reviewers --auto --max=5 --using_blame etc, depends only on our requirements.
I'm almost sure that good way is automatic way as Eyal mentioned, I know it might make a little mess but will be easier for new contributor. Why almost? Because what with a situation where all of potential reviewers are not interested to review patch?
Maybe we should try to find "the most reviewing" people, maybe it will be good way? That is another method of adding reviewers, see my suggestion in the thread which I addressed the way we should implement the algorithms for adding reviewers, so that it will be more generic.
BR, Tomek
-----Original Message----- From: Maor Lipchuk [mailto:mlipchuk@redhat.com] Sent: Tuesday, March 11, 2014 8:40 PM To: Meital Bourvine; Eyal Edri Cc: Tomasz Kołek; users@ovirt.org; infra Subject: Re: [Users] [GSOC][Gerrit] add potential reviewers - questions
Hi Tomasz,
I'm very please to hear that you are interested in the GSOC project, see my answers inline.
It's great to see that you know the material pretty well, I'm interested to hear some more ideas and feedbacks. I will start a thread soon (Maybe also schedule a call) once getting more feedbacks from other contributors, so we can brain storm some more and get practical with it. If you have any more questions, please don't hesitate to ask me.
thanks, Maor
On 03/11/2014 04:53 PM, Meital Bourvine wrote:
Eyal,
I think that he's asking about [1].
As far as I understand, the idea is to automatically get the reviewers names, with commands like `git blame`, or from gerrit - get previous reviewers to the same file/module...
Adding Maor, since he might have some additional info.
[1] http://www.ovirt.org/Summer_of_Code#Idea:_Gerrit_add_potential_reviewe rs
----- Original Message -----
From: "Eyal Edri" <eedri@redhat.com> To: "Tomasz Kołek" <tomasz-kolek@o2.pl> Cc: users@ovirt.org, "infra" <infra@ovirt.org> Sent: Tuesday, March 11, 2014 4:37:22 PM Subject: Re: [Users] [GSOC][Gerrit] add potential reviewers - questions
----- Original Message -----
From: "Tomasz Kołek" <tomasz-kolek@o2.pl> To: users@ovirt.org Sent: Monday, March 10, 2014 9:56:28 PM Subject: [Users] [GSOC][Gerrit] add potential reviewers - questions
Hi
I'd like to contribute (within GSOC) in idea: Gerrit add potential reviewers.
Maybe at first I’ll introduce myself. I'm Tomek, student from Poland. I study Computer Science at University of Wroclaw. The next year will be last year of my first degree study, I hope. As a python programmer I'm working since one year at Nokia Solutions and Networks (don't worry I intend to change my job to another or to participation at GSOC). Every day at work and school I'm using version control system (Git and SVN). At work we were using to Gerrit as a review system but currently we're using JIRA to report review statuses. I love spend my free time in mountains (mainly polish - Tatras mountains). That's all about me.
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? Adding a plugin to gerrit is an interesting idea although it will require the administrator to add it in the server, and I want that the operation will be more available to any submitter in any project. I think that we can address it in a later phase though. 2. Potential reviewers should agree that they want to review? no, since gerrit does not question the reviewer if he wants to be added or not I think we should keep this behaviour as well.
although as I see it, the operation of adding the reviewers, will not be completely automatic for the submitter, we should let the submitter of the patch to choose which reviewers he wants to add, very similar to the screen being used when doing a rebase in git. (see attached file add_reviewers.png)
3. We can have more than one accepted reviewer? Yes, that is correct.
Hi Tomasz, 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?
cause if you mean by adding reviewers to a patch, that's easily done by just clicking the '+' sign on each patch. those reviewers should have logged in at least once to gerrit.ovirt.org in order to be in the list of potential viewers, and they don't require any special permissions to review with +1/-1 any patch sent. you can add as many reviewers as you want to a patch.
does that answer your question?
Eyal.
I know above questions might seem chaotic but I think answers allow me to better understanding Yours needments
Best regards Tomasz Kolek
_______________________________________________ Users mailing list Users@ovirt.org http://lists.ovirt.org/mailman/listinfo/users
_______________________________________________ Users mailing list Users@ovirt.org http://lists.ovirt.org/mailman/listinfo/users
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?
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? _______________________________________________ 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 is required to +2 it before we consider it acked.
----- Original Message -----
From: "Itamar Heim" <iheim@redhat.com> To: "Eyal Edri" <eedri@redhat.com>, "Tomasz Kołek" <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 - 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? _______________________________________________ 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 is required to +2 it before we consider it acked.
won't it require maintaining static lists of people per file/path/project?
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łek" <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 - 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? _______________________________________________ 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 is 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 file based)
----- Original Message -----
From: "Itamar Heim" <iheim@redhat.com> To: "Eyal Edri" <eedri@redhat.com> Cc: users@ovirt.org, "Tomasz Kołek" <tomasz-kolek@o2.pl>, "infra" <infra@ovirt.org> Sent: Tuesday, March 11, 2014 5:20:29 PM Subject: Re: [Users] [GSOC][Gerrit] add potential reviewers - questions
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łek" <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 - 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? _______________________________________________ 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 is 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 file based)
Extending Ewoud's idea, we can also check the list of people committing (rather than reviewing) changes to each file in the change. -- Didi
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łek" <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 - 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? _______________________________________________ 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 is 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 file based) I think it could be done automatically by analysing the file and see who 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 familiar 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 the 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 patch that was blamed.
_______________________________________________ Users mailing list Users@ovirt.org http://lists.ovirt.org/mailman/listinfo/users
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łek" <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 - 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? _______________________________________________ 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 is 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 file based) I think it could be done automatically by analysing the file and see who 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 familiar 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 the 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 patch 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 can't do this per repo for the engine/vdsm. we can do this per repo for the other repo's probably (though solving the folder/file approach would cover the simpler repos as a private case). yes, it will require some work, maybe some moving around of files to make this easier by folders (topics) which should be relevant anyway.
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--
----- Original Message -----
From: "David Caro" <dcaroest@redhat.com> To: "Itamar Heim" <iheim@redhat.com> Cc: "Maor Lipchuk" <mlipchuk@redhat.com>, users@ovirt.org, "Tomasz Kołek" <tomasz-kolek@o2.pl>, "infra" <infra@ovirt.org> Sent: Wednesday, March 12, 2014 11:01:21 AM Subject: Re: [Users] [GSOC][Gerrit] add potential reviewers - questions
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łek" <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 - 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?
Maybe it will be worth to use the information we have in Bugzilla here: We can browse the BZ that were closed/verified in the last XXX days Per BZ , we know which patches are involved, who reviewed the patches, which files were changed, when files were changed and the rank of the change (number of lines changed) I believe that from this information we can compose a simple ranking algorithm that its output will be a list of N potential reviewers for the patch. Since we can aggregate the above information for all files related to the patch we want to add reviewers, we can have this set for the whole patch. This information should be processed and stored each N days and gerrit will be able to use it.
> _______________________________________________ > 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 is 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 file based) I think it could be done automatically by analysing the file and see who 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 familiar 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 the 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 patch 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 can't do this per repo for the engine/vdsm. we can do this per repo for the other repo's probably (though solving the folder/file approach would cover the simpler repos as a private case).
yes, it will require some work, maybe some moving around of files to make 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 root, instead of having the ownership information distributed throughout the files/directories. That way you'll know where to look at to check/modify the ownership as opposed to having to walk all the files and upper directories.
Also, adding all that ownership logic to gerrit might not be easy, as it's not meant to go checking the source of the repositories looking for configuration. We might want to take a look (again) to zuul, the tool that openstack uses as gateway to trigger jenkins jobs and merge 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
_______________________________________________ Infra mailing list Infra@ovirt.org http://lists.ovirt.org/mailman/listinfo/infra
On 03/12/2014 04:57 PM, Eli Mesika wrote:
----- Original Message -----
From: "David Caro" <dcaroest@redhat.com> To: "Itamar Heim" <iheim@redhat.com> Cc: "Maor Lipchuk" <mlipchuk@redhat.com>, users@ovirt.org, "Tomasz Kołek" <tomasz-kolek@o2.pl>, "infra" <infra@ovirt.org> Sent: Wednesday, March 12, 2014 11:01:21 AM Subject: Re: [Users] [GSOC][Gerrit] add potential reviewers - questions
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łek" > <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 - 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?
Maybe it will be worth to use the information we have in Bugzilla here:
We can browse the BZ that were closed/verified in the last XXX days Per BZ , we know which patches are involved, who reviewed the patches, which files were changed, when files were changed and the rank of the change (number of lines changed) I believe that from this information we can compose a simple ranking algorithm that its output will be a list of N potential reviewers for the patch. Since we can aggregate the above information for all files related to the patch we want to add reviewers, we can have this set for the whole patch. This information should be processed and stored each N days and gerrit will be able to use it.
why are we trying to invent hueristics, instead of declaring clear ownership?
>> _______________________________________________ >> 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 is > 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 file based) I think it could be done automatically by analysing the file and see who 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 familiar 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 the 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 patch 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 can't do this per repo for the engine/vdsm. we can do this per repo for the other repo's probably (though solving the folder/file approach would cover the simpler repos as a private case).
yes, it will require some work, maybe some moving around of files to make 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 root, instead of having the ownership information distributed throughout the files/directories. That way you'll know where to look at to check/modify the ownership as opposed to having to walk all the files and upper directories.
Also, adding all that ownership logic to gerrit might not be easy, as it's not meant to go checking the source of the repositories looking for configuration. We might want to take a look (again) to zuul, the tool that openstack uses as gateway to trigger jenkins jobs and merge 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
_______________________________________________ Infra mailing list Infra@ovirt.org http://lists.ovirt.org/mailman/listinfo/infra
----- Original Message -----
From: "Itamar Heim" <iheim@redhat.com> To: "Eli Mesika" <emesika@redhat.com>, users@ovirt.org Cc: "Maor Lipchuk" <mlipchuk@redhat.com>, "Tomasz Kołek" <tomasz-kolek@o2.pl>, "infra" <infra@ovirt.org> Sent: Wednesday, March 12, 2014 5:52:25 PM Subject: Re: [Users] [GSOC][Gerrit] add potential reviewers - questions
On 03/12/2014 04:57 PM, Eli Mesika wrote:
----- Original Message -----
From: "David Caro" <dcaroest@redhat.com> To: "Itamar Heim" <iheim@redhat.com> Cc: "Maor Lipchuk" <mlipchuk@redhat.com>, users@ovirt.org, "Tomasz Kołek" <tomasz-kolek@o2.pl>, "infra" <infra@ovirt.org> Sent: Wednesday, March 12, 2014 11:01:21 AM Subject: Re: [Users] [GSOC][Gerrit] add potential reviewers - questions
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łek" >> <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 - >> 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?
Maybe it will be worth to use the information we have in Bugzilla here:
We can browse the BZ that were closed/verified in the last XXX days Per BZ , we know which patches are involved, who reviewed the patches, which files were changed, when files were changed and the rank of the change (number of lines changed) I believe that from this information we can compose a simple ranking algorithm that its output will be a list of N potential reviewers for the patch. Since we can aggregate the above information for all files related to the patch we want to add reviewers, we can have this set for the whole patch. This information should be processed and stored each N days and gerrit will be able to use it.
why are we trying to invent hueristics, instead of declaring clear ownership?
Reminder: my source metadata plan that requires cooperation. Each source and component should have an explicit ownership up into bug database. I won't repeat it now, it is available at archives.
>>> _______________________________________________ >>> 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 is >> 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 file based) I think it could be done automatically by analysing the file and see who 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 familiar 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 the 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 patch 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 can't do this per repo for the engine/vdsm. we can do this per repo for the other repo's probably (though solving the folder/file approach would cover the simpler repos as a private case).
yes, it will require some work, maybe some moving around of files to make 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 root, instead of having the ownership information distributed throughout the files/directories. That way you'll know where to look at to check/modify the ownership as opposed to having to walk all the files and upper directories.
Also, adding all that ownership logic to gerrit might not be easy, as it's not meant to go checking the source of the repositories looking for configuration. We might want to take a look (again) to zuul, the tool that openstack uses as gateway to trigger jenkins jobs and merge 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
_______________________________________________ Infra mailing list Infra@ovirt.org http://lists.ovirt.org/mailman/listinfo/infra
_______________________________________________ Infra mailing list Infra@ovirt.org http://lists.ovirt.org/mailman/listinfo/infra
On 03/12/2014 05:57 PM, Alon Bar-Lev wrote:
----- Original Message -----
From: "Itamar Heim" <iheim@redhat.com> To: "Eli Mesika" <emesika@redhat.com>, users@ovirt.org Cc: "Maor Lipchuk" <mlipchuk@redhat.com>, "Tomasz Kołek" <tomasz-kolek@o2.pl>, "infra" <infra@ovirt.org> Sent: Wednesday, March 12, 2014 5:52:25 PM Subject: Re: [Users] [GSOC][Gerrit] add potential reviewers - questions
On 03/12/2014 04:57 PM, Eli Mesika wrote:
----- Original Message -----
From: "David Caro" <dcaroest@redhat.com> To: "Itamar Heim" <iheim@redhat.com> Cc: "Maor Lipchuk" <mlipchuk@redhat.com>, users@ovirt.org, "Tomasz Kołek" <tomasz-kolek@o2.pl>, "infra" <infra@ovirt.org> Sent: Wednesday, March 12, 2014 11:01:21 AM Subject: Re: [Users] [GSOC][Gerrit] add potential reviewers - questions
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łek" >>> <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 - >>> 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?
Maybe it will be worth to use the information we have in Bugzilla here:
We can browse the BZ that were closed/verified in the last XXX days Per BZ , we know which patches are involved, who reviewed the patches, which files were changed, when files were changed and the rank of the change (number of lines changed) I believe that from this information we can compose a simple ranking algorithm that its output will be a list of N potential reviewers for the patch. Since we can aggregate the above information for all files related to the patch we want to add reviewers, we can have this set for the whole patch. This information should be processed and stored each N days and gerrit will be able to use it.
why are we trying to invent hueristics, instead of declaring clear ownership?
Reminder: my source metadata plan that requires cooperation.
Each source and component should have an explicit ownership up into bug database.
I won't repeat it now, it is available at archives.
I think we are discussing two different issues here: The first one considers the GSOC project, of adding reviewers automatically to a patch, this can be done in many different heuristics depending what the submitter prefers (use blame, maintainers who acked the patches, bugs, list of owners to a file and so on). The second issue is adding and maintain a list of owners by files/folders in oVirt. this is a specific use case to be used by the "add potential reviewers" project.
>>>> _______________________________________________ >>>> 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 is >>> 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 > file > based) I think it could be done automatically by analysing the file and see who 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 familiar 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 the 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 patch 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 can't do this per repo for the engine/vdsm. we can do this per repo for the other repo's probably (though solving the folder/file approach would cover the simpler repos as a private case).
yes, it will require some work, maybe some moving around of files to make 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 root, instead of having the ownership information distributed throughout the files/directories. That way you'll know where to look at to check/modify the ownership as opposed to having to walk all the files and upper directories.
Also, adding all that ownership logic to gerrit might not be easy, as it's not meant to go checking the source of the repositories looking for configuration. We might want to take a look (again) to zuul, the tool that openstack uses as gateway to trigger jenkins jobs and merge 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
_______________________________________________ Infra mailing list Infra@ovirt.org http://lists.ovirt.org/mailman/listinfo/infra
_______________________________________________ Infra mailing list Infra@ovirt.org http://lists.ovirt.org/mailman/listinfo/infra
----- Original Message -----
From: "Maor Lipchuk" <mlipchuk@redhat.com> To: "Alon Bar-Lev" <alonbl@redhat.com>, "Itamar Heim" <iheim@redhat.com> Cc: "Eli Mesika" <emesika@redhat.com>, users@ovirt.org, "Tomasz Kołek" <tomasz-kolek@o2.pl>, "infra" <infra@ovirt.org> Sent: Wednesday, March 12, 2014 7:29:40 PM Subject: Re: [Users] [GSOC][Gerrit] add potential reviewers - questions
On 03/12/2014 05:57 PM, Alon Bar-Lev wrote:
----- Original Message -----
From: "Itamar Heim" <iheim@redhat.com> To: "Eli Mesika" <emesika@redhat.com>, users@ovirt.org Cc: "Maor Lipchuk" <mlipchuk@redhat.com>, "Tomasz Kołek" <tomasz-kolek@o2.pl>, "infra" <infra@ovirt.org> Sent: Wednesday, March 12, 2014 5:52:25 PM Subject: Re: [Users] [GSOC][Gerrit] add potential reviewers - questions
On 03/12/2014 04:57 PM, Eli Mesika wrote:
----- Original Message -----
From: "David Caro" <dcaroest@redhat.com> To: "Itamar Heim" <iheim@redhat.com> Cc: "Maor Lipchuk" <mlipchuk@redhat.com>, users@ovirt.org, "Tomasz Kołek" <tomasz-kolek@o2.pl>, "infra" <infra@ovirt.org> Sent: Wednesday, March 12, 2014 11:01:21 AM Subject: Re: [Users] [GSOC][Gerrit] add potential reviewers - questions
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łek" >>>> <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 - >>>> 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?
Maybe it will be worth to use the information we have in Bugzilla here:
We can browse the BZ that were closed/verified in the last XXX days Per BZ , we know which patches are involved, who reviewed the patches, which files were changed, when files were changed and the rank of the change (number of lines changed) I believe that from this information we can compose a simple ranking algorithm that its output will be a list of N potential reviewers for the patch. Since we can aggregate the above information for all files related to the patch we want to add reviewers, we can have this set for the whole patch. This information should be processed and stored each N days and gerrit will be able to use it.
why are we trying to invent hueristics, instead of declaring clear ownership?
Reminder: my source metadata plan that requires cooperation.
Each source and component should have an explicit ownership up into bug database.
I won't repeat it now, it is available at archives.
I think we are discussing two different issues here: The first one considers the GSOC project, of adding reviewers automatically to a patch, this can be done in many different heuristics depending what the submitter prefers (use blame, maintainers who acked the patches, bugs, list of owners to a file and so on).
The second issue is adding and maintain a list of owners by files/folders in oVirt. this is a specific use case to be used by the "add potential reviewers" project.
There is no such think as "reviewer" there is component maintainer. Random reviewers are irrelevant. Each GSOC participate should have a mentor, this mentor should be the maintainer of the relevant component. The mentor is responsible for the process, while the participate is providing the technical work.
>>>>> _______________________________________________ >>>>> 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 >>>> is >>>> 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 >> file >> based) > I think it could be done automatically by analysing the file and see > who > 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 > familiar > 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 > the > 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 > patch > 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 can't do this per repo for the engine/vdsm. we can do this per repo for the other repo's probably (though solving the folder/file approach would cover the simpler repos as a private case).
yes, it will require some work, maybe some moving around of files to make 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 root, instead of having the ownership information distributed throughout the files/directories. That way you'll know where to look at to check/modify the ownership as opposed to having to walk all the files and upper directories.
Also, adding all that ownership logic to gerrit might not be easy, as it's not meant to go checking the source of the repositories looking for configuration. We might want to take a look (again) to zuul, the tool that openstack uses as gateway to trigger jenkins jobs and merge 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
_______________________________________________ Infra mailing list Infra@ovirt.org http://lists.ovirt.org/mailman/listinfo/infra
_______________________________________________ Infra mailing list Infra@ovirt.org http://lists.ovirt.org/mailman/listinfo/infra
On 03/12/2014 07:29 PM, Maor Lipchuk wrote:
On 03/12/2014 05:57 PM, Alon Bar-Lev wrote:
----- Original Message -----
From: "Itamar Heim" <iheim@redhat.com> To: "Eli Mesika" <emesika@redhat.com>, users@ovirt.org Cc: "Maor Lipchuk" <mlipchuk@redhat.com>, "Tomasz Kołek" <tomasz-kolek@o2.pl>, "infra" <infra@ovirt.org> Sent: Wednesday, March 12, 2014 5:52:25 PM Subject: Re: [Users] [GSOC][Gerrit] add potential reviewers - questions
On 03/12/2014 04:57 PM, Eli Mesika wrote:
----- Original Message -----
From: "David Caro" <dcaroest@redhat.com> To: "Itamar Heim" <iheim@redhat.com> Cc: "Maor Lipchuk" <mlipchuk@redhat.com>, users@ovirt.org, "Tomasz Kołek" <tomasz-kolek@o2.pl>, "infra" <infra@ovirt.org> Sent: Wednesday, March 12, 2014 11:01:21 AM Subject: Re: [Users] [GSOC][Gerrit] add potential reviewers - questions
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łek" >>>> <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 - >>>> 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?
Maybe it will be worth to use the information we have in Bugzilla here:
We can browse the BZ that were closed/verified in the last XXX days Per BZ , we know which patches are involved, who reviewed the patches, which files were changed, when files were changed and the rank of the change (number of lines changed) I believe that from this information we can compose a simple ranking algorithm that its output will be a list of N potential reviewers for the patch. Since we can aggregate the above information for all files related to the patch we want to add reviewers, we can have this set for the whole patch. This information should be processed and stored each N days and gerrit will be able to use it.
why are we trying to invent hueristics, instead of declaring clear ownership?
Reminder: my source metadata plan that requires cooperation.
Each source and component should have an explicit ownership up into bug database.
I won't repeat it now, it is available at archives.
I think we are discussing two different issues here: The first one considers the GSOC project, of adding reviewers automatically to a patch, this can be done in many different heuristics depending what the submitter prefers (use blame, maintainers who acked the patches, bugs, list of owners to a file and so on).
that's the point. we should be using heuristics, rather ownership. this could still be done via a GSOC project for implementing the gerrit logic to do so.
The second issue is adding and maintain a list of owners by files/folders in oVirt. this is a specific use case to be used by the "add potential reviewers" project.
>>>>> _______________________________________________ >>>>> 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 is >>>> 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 >> file >> based) > I think it could be done automatically by analysing the file and see who > 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 familiar > 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 the > 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 patch > 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 can't do this per repo for the engine/vdsm. we can do this per repo for the other repo's probably (though solving the folder/file approach would cover the simpler repos as a private case).
yes, it will require some work, maybe some moving around of files to make 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 root, instead of having the ownership information distributed throughout the files/directories. That way you'll know where to look at to check/modify the ownership as opposed to having to walk all the files and upper directories.
Also, adding all that ownership logic to gerrit might not be easy, as it's not meant to go checking the source of the repositories looking for configuration. We might want to take a look (again) to zuul, the tool that openstack uses as gateway to trigger jenkins jobs and merge 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
_______________________________________________ Infra mailing list Infra@ovirt.org http://lists.ovirt.org/mailman/listinfo/infra
_______________________________________________ Infra mailing list Infra@ovirt.org http://lists.ovirt.org/mailman/listinfo/infra
----- Original Message -----
From: "Itamar Heim" <iheim@redhat.com> To: "Maor Lipchuk" <mlipchuk@redhat.com>, "Alon Bar-Lev" <alonbl@redhat.com> Cc: "Eli Mesika" <emesika@redhat.com>, users@ovirt.org, "Tomasz Kołek" <tomasz-kolek@o2.pl>, "infra" <infra@ovirt.org> Sent: Wednesday, March 12, 2014 9:29:28 PM Subject: Re: [Users] [GSOC][Gerrit] add potential reviewers - questions
On 03/12/2014 07:29 PM, Maor Lipchuk wrote:
On 03/12/2014 05:57 PM, Alon Bar-Lev wrote:
----- Original Message -----
From: "Itamar Heim" <iheim@redhat.com> To: "Eli Mesika" <emesika@redhat.com>, users@ovirt.org Cc: "Maor Lipchuk" <mlipchuk@redhat.com>, "Tomasz Kołek" <tomasz-kolek@o2.pl>, "infra" <infra@ovirt.org> Sent: Wednesday, March 12, 2014 5:52:25 PM Subject: Re: [Users] [GSOC][Gerrit] add potential reviewers - questions
On 03/12/2014 04:57 PM, Eli Mesika wrote:
----- Original Message -----
From: "David Caro" <dcaroest@redhat.com> To: "Itamar Heim" <iheim@redhat.com> Cc: "Maor Lipchuk" <mlipchuk@redhat.com>, users@ovirt.org, "Tomasz Kołek" <tomasz-kolek@o2.pl>, "infra" <infra@ovirt.org> Sent: Wednesday, March 12, 2014 11:01:21 AM Subject: Re: [Users] [GSOC][Gerrit] add potential reviewers - questions
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łek" >>>>> <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 - >>>>> 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?
Maybe it will be worth to use the information we have in Bugzilla here:
We can browse the BZ that were closed/verified in the last XXX days Per BZ , we know which patches are involved, who reviewed the patches, which files were changed, when files were changed and the rank of the change (number of lines changed) I believe that from this information we can compose a simple ranking algorithm that its output will be a list of N potential reviewers for the patch. Since we can aggregate the above information for all files related to the patch we want to add reviewers, we can have this set for the whole patch. This information should be processed and stored each N days and gerrit will be able to use it.
why are we trying to invent hueristics, instead of declaring clear ownership?
Reminder: my source metadata plan that requires cooperation.
Each source and component should have an explicit ownership up into bug database.
I won't repeat it now, it is available at archives.
I think we are discussing two different issues here: The first one considers the GSOC project, of adding reviewers automatically to a patch, this can be done in many different heuristics depending what the submitter prefers (use blame, maintainers who acked the patches, bugs, list of owners to a file and so on).
that's the point. we should be using heuristics, rather ownership.
I want heuristics to solve bugs as well. Maybe for each issue I will use heuristic and wait for X days for someone else to fix? I would like to see how you build a building using heuristic reviews.
this could still be done via a GSOC project for implementing the gerrit logic to do so.
The second issue is adding and maintain a list of owners by files/folders in oVirt. this is a specific use case to be used by the "add potential reviewers" project.
>>>>>> _______________________________________________ >>>>>> 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 >>>>> is >>>>> 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 >>> file >>> based) >> I think it could be done automatically by analysing the file and see >> who >> 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 >> familiar >> 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 >> the >> 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 >> patch >> 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 > can't > do > this per repo for the engine/vdsm. we can do this per repo for the > other > repo's probably (though solving the folder/file approach would cover > the > simpler repos as a private case). > > yes, it will require some work, maybe some moving around of files to > make > 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 root, instead of having the ownership information distributed throughout the files/directories. That way you'll know where to look at to check/modify the ownership as opposed to having to walk all the files and upper directories.
Also, adding all that ownership logic to gerrit might not be easy, as it's not meant to go checking the source of the repositories looking for configuration. We might want to take a look (again) to zuul, the tool that openstack uses as gateway to trigger jenkins jobs and merge 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
_______________________________________________ Infra mailing list Infra@ovirt.org http://lists.ovirt.org/mailman/listinfo/infra
_______________________________________________ Infra mailing list Infra@ovirt.org http://lists.ovirt.org/mailman/listinfo/infra
On 03/12/2014 09:42 PM, Alon Bar-Lev wrote:
----- Original Message -----
From: "Itamar Heim" <iheim@redhat.com> To: "Maor Lipchuk" <mlipchuk@redhat.com>, "Alon Bar-Lev" <alonbl@redhat.com> Cc: "Eli Mesika" <emesika@redhat.com>, users@ovirt.org, "Tomasz Kołek" <tomasz-kolek@o2.pl>, "infra" <infra@ovirt.org> Sent: Wednesday, March 12, 2014 9:29:28 PM Subject: Re: [Users] [GSOC][Gerrit] add potential reviewers - questions
On 03/12/2014 07:29 PM, Maor Lipchuk wrote:
On 03/12/2014 05:57 PM, Alon Bar-Lev wrote:
----- Original Message -----
From: "Itamar Heim" <iheim@redhat.com> To: "Eli Mesika" <emesika@redhat.com>, users@ovirt.org Cc: "Maor Lipchuk" <mlipchuk@redhat.com>, "Tomasz Kołek" <tomasz-kolek@o2.pl>, "infra" <infra@ovirt.org> Sent: Wednesday, March 12, 2014 5:52:25 PM Subject: Re: [Users] [GSOC][Gerrit] add potential reviewers - questions
On 03/12/2014 04:57 PM, Eli Mesika wrote:
----- Original Message ----- > From: "David Caro" <dcaroest@redhat.com> > To: "Itamar Heim" <iheim@redhat.com> > Cc: "Maor Lipchuk" <mlipchuk@redhat.com>, users@ovirt.org, "Tomasz > Kołek" > <tomasz-kolek@o2.pl>, "infra" > <infra@ovirt.org> > Sent: Wednesday, March 12, 2014 11:01:21 AM > Subject: Re: [Users] [GSOC][Gerrit] add potential reviewers - questions > > 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łek" >>>>>> <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 - >>>>>> 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?
Maybe it will be worth to use the information we have in Bugzilla here:
We can browse the BZ that were closed/verified in the last XXX days Per BZ , we know which patches are involved, who reviewed the patches, which files were changed, when files were changed and the rank of the change (number of lines changed) I believe that from this information we can compose a simple ranking algorithm that its output will be a list of N potential reviewers for the patch. Since we can aggregate the above information for all files related to the patch we want to add reviewers, we can have this set for the whole patch. This information should be processed and stored each N days and gerrit will be able to use it.
why are we trying to invent hueristics, instead of declaring clear ownership?
Reminder: my source metadata plan that requires cooperation.
Each source and component should have an explicit ownership up into bug database.
I won't repeat it now, it is available at archives.
I think we are discussing two different issues here: The first one considers the GSOC project, of adding reviewers automatically to a patch, this can be done in many different heuristics depending what the submitter prefers (use blame, maintainers who acked the patches, bugs, list of owners to a file and so on).
that's the point. we should be using heuristics, rather ownership.
obvioulsy i meant here: we should *not* be using heuristics, rather ownership.
I want heuristics to solve bugs as well. Maybe for each issue I will use heuristic and wait for X days for someone else to fix? I would like to see how you build a building using heuristic reviews.
this could still be done via a GSOC project for implementing the gerrit logic to do so.
The second issue is adding and maintain a list of owners by files/folders in oVirt. this is a specific use case to be used by the "add potential reviewers" project.
>>>>>>> _______________________________________________ >>>>>>> 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 >>>>>> is >>>>>> 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 >>>> file >>>> based) >>> I think it could be done automatically by analysing the file and see >>> who >>> 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 >>> familiar >>> 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 >>> the >>> 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 >>> patch >>> 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 >> can't >> do >> this per repo for the engine/vdsm. we can do this per repo for the >> other >> repo's probably (though solving the folder/file approach would cover >> the >> simpler repos as a private case). >> >> yes, it will require some work, maybe some moving around of files to >> make >> 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 > root, instead of having the ownership information distributed > throughout the files/directories. That way you'll know where to look at > to check/modify the ownership as opposed to having to walk all the > files and upper directories. > > Also, adding all that ownership logic to gerrit might not be easy, as > it's not meant to go checking the source of the repositories looking > for configuration. We might want to take a look (again) to zuul, the > tool that openstack uses as gateway to trigger jenkins jobs and merge > 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 > > > _______________________________________________ > Infra mailing list > Infra@ovirt.org > http://lists.ovirt.org/mailman/listinfo/infra >
_______________________________________________ Infra mailing list Infra@ovirt.org http://lists.ovirt.org/mailman/listinfo/infra
This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --8g2T7IQmS1E8OmSlnIBno6eqBl8hkAFtD Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On Tue 11 Mar 2014 04:14:26 PM CET, 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 - question= s
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 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 revi= ewers? 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 t= o 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 =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 maintain= er for that file. A more complex solution could improve on that and limi= t 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/formatting patches can skew the line count.
In short, I think an entire thesis could be written on the optimal wa= y 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 is required to +2 it before we consider it acked.
won't it require maintaining static lists of people per file/path/proje= ct? _______________________________________________ Infra mailing list Infra@ovirt.org http://lists.ovirt.org/mailman/listinfo/infra
We already discussed that, in the project, and one of the solutions=20 proposed was to add metadata on each file/directory about the=20 maintainers for that file/directory. For what I've seen in other=20 projects (mainly openstack), they do that per repository, and not per=20 file, that simplifies the maintainers selection greatly (having it on=20 gerrit config so you don't have to check the code to decide who has=20 merge rights). But that might be another subject. About dynamically adding reviewers, it should be fairly easy to do=20 using a small gerrit hook to run on patch-submitted. But gerrit will=20 fail if the user is not already created in gerrit so not really=20 allowing non-registered users to get in. But I'm not sure if it's=20 'polite' to add someone as a reviewer if he did not previously agreed=20 to it, specially when emails will be sent to him. And as Ewoud says, selecting which commiters are considered possible=20 reviewers gives enough talk for a thesis by itself. -- 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 --8g2T7IQmS1E8OmSlnIBno6eqBl8hkAFtD 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 iQEcBAEBAgAGBQJTHzxiAAoJEEBxx+HSYmnD2ZEH/1i8of7ywJaIwKKyxzgyYp2H AUlZRkqCbdZTkVrMSIFsXikuRnEVeJY0eXVd5G4mco2Qf7962+dpW2xjLctJL8T0 YaLXiQUR+TvpnBp5OvB0870hhKo9ql010KjijjediMceswENSV6fwciQn+zvdUfe MkCD6/I5aB4vwLGm7CUIB64lLeaSbYGoAUJYc3XkH+czKmSJce1D4JD3xCDufLLE NDu0jdcHizss0czsWzwG8k8twDE/MVgaLmd0xiU3Mvjg3pkQ6h7mUQMe7spRWpJM FOSI+MBek1q+mxtcbsuGpPvyT5WqyoKA6XzYWrWdclvCfzQiYhRJ1eQTjUIgKcE= =7A1Y -----END PGP SIGNATURE----- --8g2T7IQmS1E8OmSlnIBno6eqBl8hkAFtD--
participants (10)
-
Alon Bar-Lev -
David Caro -
Eli Mesika -
Ewoud Kohl van Wijngaarden -
Eyal Edri -
Itamar Heim -
Maor Lipchuk -
Meital Bourvine -
Tomasz Kołek -
Yedidyah Bar David