Re: [Gluster-Maintainers] [Gluster-devel] 'Reviewd-by' tag for commits

[Jose A. Rivera]

On Fri, Oct 14, 2016 at 4:44 AM, Niels de Vos  wrote:
> On Fri, Oct 14, 2016 at 02:21:23PM +0530, Nigel Babu wrote:
>> I've said on this thread before, none of this is easy to do. It needs us to
>> fork Gerrit to make our own changes. I would argue that depending on the
>> data from the commit message is folly.
>
> Eventhough we all seem to agree that statistics based on commit messages
> is not correct, it looks like it is an incentive to get reviewing valued
> more. We need to promote the reviewing work somehow, and this is one way
> to do it.
>
> Forking Gerrit is surely not the right thing. But could it not get
> discussed with the rest of the Gerrit community? I hope that the Gerrit
> admins follow the Gerrit project and know how to report feature requests
> or such?
>
> Thanks,
> Niels

This sounds like something that should be doable via a Gerrit plugin,
such that we wouldn't need to fork the entire project. That said, my
previous experience in interacting with the Gerrit maintainers was
definitely one of "willing to provide answers to questions, less than
willing to pay attention to feature requests".

--Jose

>>
>> On Fri, Oct 14, 2016 at 12:23 PM, Niels de Vos  wrote:
>>
>> > On Thu, Oct 13, 2016 at 11:01:43PM +0530, Pranith Kumar Karampuri wrote:
>> > > On Thu, Oct 6, 2016 at 1:49 AM, Michael Adam  wrote:
>> > >
>> > > > On 2016-10-05 at 09:45 -0400, Ira Cooper wrote:
>> > > > > "Feedback-given-by: "
>> > > >
>> > >
>> > > Niels/Nigel,
>> > >Is this easier to do?
>> >
>> > No idea if this can be done by a Gerrit configuration, I'm not an admin
>> > there :)
>> >
>> > I suspect Gerrit gives the option to run a script after someone pressed
>> > the [submit] button for merging, and before the actual commit is pushed
>> > into the branch. If there is no config option, such a hook-script could
>> > be made to work. But, my Gerrit experience on that level is
>> > non-existent, so I can be completely wrong.
>> >
>> > Niels
>> >
>> > >
>> > >
>> > > >
>> > > > I like that one - thanks! :-)
>> > > >
>> > > > Michael
>> > > >
>> > > > > - Original Message -
>> > > > > > On 2016-09-30 at 17:52 +0200, Niels de Vos wrote:
>> > > > > > > On Fri, Sep 30, 2016 at 08:50:12PM +0530, Ravishankar N wrote:
>> > > > > > > > On 09/30/2016 06:38 PM, Niels de Vos wrote:
>> > > > > > > > > On Fri, Sep 30, 2016 at 07:11:51AM +0530, Pranith Kumar
>> > Karampuri
>> > > > > > > > > wrote:
>> > > > > > > ...
>> > > > > > > > > Maybe we can add an additional tag that mentions all the
>> > people
>> > > > that
>> > > > > > > > > did do reviews of older versions of the patch. Not sure what
>> > the
>> > > > tag
>> > > > > > > > > would be, maybe just CC?
>> > > > > > > > It depends on what tags would be processed to obtain
>> > statistics on
>> > > > review
>> > > > > > > > contributions.
>> > > > > > >
>> > > > > > > Real statistics would come from Gerrit, not from the 'git log'
>> > > > output.
>> > > > > > > We do have a ./extras/who-wrote-glusterfs/ in the sources, but
>> > that
>> > > > is
>> > > > > > > only to get an idea about the changes that were made and should
>> > not
>> > > > be
>> > > > > > > used for serious statistics.
>> > > > > > >
>> > > > > > > It is possible to feed the Gerrit comment-stream into things like
>> > > > > > > Elasticsearch and get an accurate impression how many reviews
>> > people
>> > > > do
>> > > > > > > (and much more). I hope we can get some contribution diagrams
>> > from
>> > > > > > > someting like this at one point.
>> > > > > > >
>> > > > > > > Would some kind of Gave-feedback tag for people that left a
>> > comment
>> > > > on
>> > > > > > > earlier versions of the patch be appreciated by others? It will
>> > show
>> > > > in
>> > > > > > > the 'git log' who was involved in some way or form.
>> > > > > >
>> > > > > > I think this would be fair.
>> > > > > >
>> > > > > > Reviewed-by tags should imho be reserved for the final
>> > > > > > incarnation of the patch. Those mean that the person named
>> > > > > > in the tag has aproved this version of the patch for getting
>> > > > > > into the official tree. A previous version of the patch can
>> > > > > > have been entirely different, so a reviewed-by for that
>> > > > > > previous version may not actually apply to the new version at all
>> > > > > > and hence create a false impression!
>> > > > > >
>> > > > > > It is also difficult to track all activities by tags,
>> > > > > > and anyone who wants to measure performance and contributions
>> > > > > > only by looking at git commit tags will not be doing several
>> > > > > > people justice. We could add 'discussed-with' or 'designed-by'
>> > > > > > tags, etc ... ;-)
>> > > > > >
>> > > > > > On a serious note, in Samba we use 'Pair-programmed-with' tags,
>> > > > > > because we do pair-programming a lot, but only one person can
>> > > > > > be an author of a git commit ...
>> > > > > >
>> > 

Re: [Gluster-Maintainers] [Gluster-devel] 'Reviewd-by' tag for commits

[Pranith Kumar Karampuri]

How do we get the following tags in the commit message?

> Smoke: Gluster Build System 
> NetBSD-regression: NetBSD Build System 
> CentOS-regression: Gluster Build System 


On Fri, Oct 14, 2016 at 3:14 PM, Niels de Vos  wrote:

> On Fri, Oct 14, 2016 at 02:21:23PM +0530, Nigel Babu wrote:
> > I've said on this thread before, none of this is easy to do. It needs us
> to
> > fork Gerrit to make our own changes. I would argue that depending on the
> > data from the commit message is folly.
>
> Eventhough we all seem to agree that statistics based on commit messages
> is not correct, it looks like it is an incentive to get reviewing valued
> more. We need to promote the reviewing work somehow, and this is one way
> to do it.
>
> Forking Gerrit is surely not the right thing. But could it not get
> discussed with the rest of the Gerrit community? I hope that the Gerrit
> admins follow the Gerrit project and know how to report feature requests
> or such?
>
> Thanks,
> Niels
>
>
> >
> > On Fri, Oct 14, 2016 at 12:23 PM, Niels de Vos 
> wrote:
> >
> > > On Thu, Oct 13, 2016 at 11:01:43PM +0530, Pranith Kumar Karampuri
> wrote:
> > > > On Thu, Oct 6, 2016 at 1:49 AM, Michael Adam 
> wrote:
> > > >
> > > > > On 2016-10-05 at 09:45 -0400, Ira Cooper wrote:
> > > > > > "Feedback-given-by: "
> > > > >
> > > >
> > > > Niels/Nigel,
> > > >Is this easier to do?
> > >
> > > No idea if this can be done by a Gerrit configuration, I'm not an admin
> > > there :)
> > >
> > > I suspect Gerrit gives the option to run a script after someone pressed
> > > the [submit] button for merging, and before the actual commit is pushed
> > > into the branch. If there is no config option, such a hook-script could
> > > be made to work. But, my Gerrit experience on that level is
> > > non-existent, so I can be completely wrong.
> > >
> > > Niels
> > >
> > > >
> > > >
> > > > >
> > > > > I like that one - thanks! :-)
> > > > >
> > > > > Michael
> > > > >
> > > > > > - Original Message -
> > > > > > > On 2016-09-30 at 17:52 +0200, Niels de Vos wrote:
> > > > > > > > On Fri, Sep 30, 2016 at 08:50:12PM +0530, Ravishankar N
> wrote:
> > > > > > > > > On 09/30/2016 06:38 PM, Niels de Vos wrote:
> > > > > > > > > > On Fri, Sep 30, 2016 at 07:11:51AM +0530, Pranith Kumar
> > > Karampuri
> > > > > > > > > > wrote:
> > > > > > > > ...
> > > > > > > > > > Maybe we can add an additional tag that mentions all the
> > > people
> > > > > that
> > > > > > > > > > did do reviews of older versions of the patch. Not sure
> what
> > > the
> > > > > tag
> > > > > > > > > > would be, maybe just CC?
> > > > > > > > > It depends on what tags would be processed to obtain
> > > statistics on
> > > > > review
> > > > > > > > > contributions.
> > > > > > > >
> > > > > > > > Real statistics would come from Gerrit, not from the 'git
> log'
> > > > > output.
> > > > > > > > We do have a ./extras/who-wrote-glusterfs/ in the sources,
> but
> > > that
> > > > > is
> > > > > > > > only to get an idea about the changes that were made and
> should
> > > not
> > > > > be
> > > > > > > > used for serious statistics.
> > > > > > > >
> > > > > > > > It is possible to feed the Gerrit comment-stream into things
> like
> > > > > > > > Elasticsearch and get an accurate impression how many reviews
> > > people
> > > > > do
> > > > > > > > (and much more). I hope we can get some contribution diagrams
> > > from
> > > > > > > > someting like this at one point.
> > > > > > > >
> > > > > > > > Would some kind of Gave-feedback tag for people that left a
> > > comment
> > > > > on
> > > > > > > > earlier versions of the patch be appreciated by others? It
> will
> > > show
> > > > > in
> > > > > > > > the 'git log' who was involved in some way or form.
> > > > > > >
> > > > > > > I think this would be fair.
> > > > > > >
> > > > > > > Reviewed-by tags should imho be reserved for the final
> > > > > > > incarnation of the patch. Those mean that the person named
> > > > > > > in the tag has aproved this version of the patch for getting
> > > > > > > into the official tree. A previous version of the patch can
> > > > > > > have been entirely different, so a reviewed-by for that
> > > > > > > previous version may not actually apply to the new version at
> all
> > > > > > > and hence create a false impression!
> > > > > > >
> > > > > > > It is also difficult to track all activities by tags,
> > > > > > > and anyone who wants to measure performance and contributions
> > > > > > > only by looking at git commit tags will not be doing several
> > > > > > > people justice. We could add 'discussed-with' or 'designed-by'
> > > > > > > tags, etc ... ;-)
> > > > > > >
> > > > > > > On a serious note, in Samba we use 'Pair-programmed-with' tags,
> > > > > > > because we do pair-programming a lot, but only one person can
> 

Re: [Gluster-Maintainers] [Gluster-devel] 'Reviewd-by' tag for commits

[Niels de Vos]

On Fri, Oct 14, 2016 at 02:21:23PM +0530, Nigel Babu wrote:
> I've said on this thread before, none of this is easy to do. It needs us to
> fork Gerrit to make our own changes. I would argue that depending on the
> data from the commit message is folly.

Eventhough we all seem to agree that statistics based on commit messages
is not correct, it looks like it is an incentive to get reviewing valued
more. We need to promote the reviewing work somehow, and this is one way
to do it.

Forking Gerrit is surely not the right thing. But could it not get
discussed with the rest of the Gerrit community? I hope that the Gerrit
admins follow the Gerrit project and know how to report feature requests
or such?

Thanks,
Niels


> 
> On Fri, Oct 14, 2016 at 12:23 PM, Niels de Vos  wrote:
> 
> > On Thu, Oct 13, 2016 at 11:01:43PM +0530, Pranith Kumar Karampuri wrote:
> > > On Thu, Oct 6, 2016 at 1:49 AM, Michael Adam  wrote:
> > >
> > > > On 2016-10-05 at 09:45 -0400, Ira Cooper wrote:
> > > > > "Feedback-given-by: "
> > > >
> > >
> > > Niels/Nigel,
> > >Is this easier to do?
> >
> > No idea if this can be done by a Gerrit configuration, I'm not an admin
> > there :)
> >
> > I suspect Gerrit gives the option to run a script after someone pressed
> > the [submit] button for merging, and before the actual commit is pushed
> > into the branch. If there is no config option, such a hook-script could
> > be made to work. But, my Gerrit experience on that level is
> > non-existent, so I can be completely wrong.
> >
> > Niels
> >
> > >
> > >
> > > >
> > > > I like that one - thanks! :-)
> > > >
> > > > Michael
> > > >
> > > > > - Original Message -
> > > > > > On 2016-09-30 at 17:52 +0200, Niels de Vos wrote:
> > > > > > > On Fri, Sep 30, 2016 at 08:50:12PM +0530, Ravishankar N wrote:
> > > > > > > > On 09/30/2016 06:38 PM, Niels de Vos wrote:
> > > > > > > > > On Fri, Sep 30, 2016 at 07:11:51AM +0530, Pranith Kumar
> > Karampuri
> > > > > > > > > wrote:
> > > > > > > ...
> > > > > > > > > Maybe we can add an additional tag that mentions all the
> > people
> > > > that
> > > > > > > > > did do reviews of older versions of the patch. Not sure what
> > the
> > > > tag
> > > > > > > > > would be, maybe just CC?
> > > > > > > > It depends on what tags would be processed to obtain
> > statistics on
> > > > review
> > > > > > > > contributions.
> > > > > > >
> > > > > > > Real statistics would come from Gerrit, not from the 'git log'
> > > > output.
> > > > > > > We do have a ./extras/who-wrote-glusterfs/ in the sources, but
> > that
> > > > is
> > > > > > > only to get an idea about the changes that were made and should
> > not
> > > > be
> > > > > > > used for serious statistics.
> > > > > > >
> > > > > > > It is possible to feed the Gerrit comment-stream into things like
> > > > > > > Elasticsearch and get an accurate impression how many reviews
> > people
> > > > do
> > > > > > > (and much more). I hope we can get some contribution diagrams
> > from
> > > > > > > someting like this at one point.
> > > > > > >
> > > > > > > Would some kind of Gave-feedback tag for people that left a
> > comment
> > > > on
> > > > > > > earlier versions of the patch be appreciated by others? It will
> > show
> > > > in
> > > > > > > the 'git log' who was involved in some way or form.
> > > > > >
> > > > > > I think this would be fair.
> > > > > >
> > > > > > Reviewed-by tags should imho be reserved for the final
> > > > > > incarnation of the patch. Those mean that the person named
> > > > > > in the tag has aproved this version of the patch for getting
> > > > > > into the official tree. A previous version of the patch can
> > > > > > have been entirely different, so a reviewed-by for that
> > > > > > previous version may not actually apply to the new version at all
> > > > > > and hence create a false impression!
> > > > > >
> > > > > > It is also difficult to track all activities by tags,
> > > > > > and anyone who wants to measure performance and contributions
> > > > > > only by looking at git commit tags will not be doing several
> > > > > > people justice. We could add 'discussed-with' or 'designed-by'
> > > > > > tags, etc ... ;-)
> > > > > >
> > > > > > On a serious note, in Samba we use 'Pair-programmed-with' tags,
> > > > > > because we do pair-programming a lot, but only one person can
> > > > > > be an author of a git commit ...
> > > > > >
> > > > > > The 'Gave-feedback' tag I do like. even though it does
> > > > > > not quite match with the foobar-by pattern of other tags.
> > > > > >
> > > > > > Michael
> > > > > >
> > > > > > ___
> > > > > > Gluster-devel mailing list
> > > > > > gluster-de...@gluster.org
> > > > > > http://www.gluster.org/mailman/listinfo/gluster-devel
> > > >
> > > > ___
> > > > maintainers mailing list
> > > > maintainers@gluster.org
> > > > 

Re: [Gluster-Maintainers] [Gluster-devel] 'Reviewd-by' tag for commits

[Nigel Babu]

I've said on this thread before, none of this is easy to do. It needs us to
fork Gerrit to make our own changes. I would argue that depending on the
data from the commit message is folly.

On Fri, Oct 14, 2016 at 12:23 PM, Niels de Vos  wrote:

> On Thu, Oct 13, 2016 at 11:01:43PM +0530, Pranith Kumar Karampuri wrote:
> > On Thu, Oct 6, 2016 at 1:49 AM, Michael Adam  wrote:
> >
> > > On 2016-10-05 at 09:45 -0400, Ira Cooper wrote:
> > > > "Feedback-given-by: "
> > >
> >
> > Niels/Nigel,
> >Is this easier to do?
>
> No idea if this can be done by a Gerrit configuration, I'm not an admin
> there :)
>
> I suspect Gerrit gives the option to run a script after someone pressed
> the [submit] button for merging, and before the actual commit is pushed
> into the branch. If there is no config option, such a hook-script could
> be made to work. But, my Gerrit experience on that level is
> non-existent, so I can be completely wrong.
>
> Niels
>
> >
> >
> > >
> > > I like that one - thanks! :-)
> > >
> > > Michael
> > >
> > > > - Original Message -
> > > > > On 2016-09-30 at 17:52 +0200, Niels de Vos wrote:
> > > > > > On Fri, Sep 30, 2016 at 08:50:12PM +0530, Ravishankar N wrote:
> > > > > > > On 09/30/2016 06:38 PM, Niels de Vos wrote:
> > > > > > > > On Fri, Sep 30, 2016 at 07:11:51AM +0530, Pranith Kumar
> Karampuri
> > > > > > > > wrote:
> > > > > > ...
> > > > > > > > Maybe we can add an additional tag that mentions all the
> people
> > > that
> > > > > > > > did do reviews of older versions of the patch. Not sure what
> the
> > > tag
> > > > > > > > would be, maybe just CC?
> > > > > > > It depends on what tags would be processed to obtain
> statistics on
> > > review
> > > > > > > contributions.
> > > > > >
> > > > > > Real statistics would come from Gerrit, not from the 'git log'
> > > output.
> > > > > > We do have a ./extras/who-wrote-glusterfs/ in the sources, but
> that
> > > is
> > > > > > only to get an idea about the changes that were made and should
> not
> > > be
> > > > > > used for serious statistics.
> > > > > >
> > > > > > It is possible to feed the Gerrit comment-stream into things like
> > > > > > Elasticsearch and get an accurate impression how many reviews
> people
> > > do
> > > > > > (and much more). I hope we can get some contribution diagrams
> from
> > > > > > someting like this at one point.
> > > > > >
> > > > > > Would some kind of Gave-feedback tag for people that left a
> comment
> > > on
> > > > > > earlier versions of the patch be appreciated by others? It will
> show
> > > in
> > > > > > the 'git log' who was involved in some way or form.
> > > > >
> > > > > I think this would be fair.
> > > > >
> > > > > Reviewed-by tags should imho be reserved for the final
> > > > > incarnation of the patch. Those mean that the person named
> > > > > in the tag has aproved this version of the patch for getting
> > > > > into the official tree. A previous version of the patch can
> > > > > have been entirely different, so a reviewed-by for that
> > > > > previous version may not actually apply to the new version at all
> > > > > and hence create a false impression!
> > > > >
> > > > > It is also difficult to track all activities by tags,
> > > > > and anyone who wants to measure performance and contributions
> > > > > only by looking at git commit tags will not be doing several
> > > > > people justice. We could add 'discussed-with' or 'designed-by'
> > > > > tags, etc ... ;-)
> > > > >
> > > > > On a serious note, in Samba we use 'Pair-programmed-with' tags,
> > > > > because we do pair-programming a lot, but only one person can
> > > > > be an author of a git commit ...
> > > > >
> > > > > The 'Gave-feedback' tag I do like. even though it does
> > > > > not quite match with the foobar-by pattern of other tags.
> > > > >
> > > > > Michael
> > > > >
> > > > > ___
> > > > > Gluster-devel mailing list
> > > > > gluster-de...@gluster.org
> > > > > http://www.gluster.org/mailman/listinfo/gluster-devel
> > >
> > > ___
> > > maintainers mailing list
> > > maintainers@gluster.org
> > > http://www.gluster.org/mailman/listinfo/maintainers
> > >
> > >
> >
> >
> > --
> > Pranith
>
> > ___
> > maintainers mailing list
> > maintainers@gluster.org
> > http://www.gluster.org/mailman/listinfo/maintainers
>
>
> ___
> maintainers mailing list
> maintainers@gluster.org
> http://www.gluster.org/mailman/listinfo/maintainers
>
>


-- 
nigelb
___
maintainers mailing list
maintainers@gluster.org
http://www.gluster.org/mailman/listinfo/maintainers

Re: [Gluster-Maintainers] [Gluster-devel] 'Reviewd-by' tag for commits

[Niels de Vos]

On Thu, Oct 13, 2016 at 11:01:43PM +0530, Pranith Kumar Karampuri wrote:
> On Thu, Oct 6, 2016 at 1:49 AM, Michael Adam  wrote:
> 
> > On 2016-10-05 at 09:45 -0400, Ira Cooper wrote:
> > > "Feedback-given-by: "
> >
> 
> Niels/Nigel,
>Is this easier to do?

No idea if this can be done by a Gerrit configuration, I'm not an admin
there :)

I suspect Gerrit gives the option to run a script after someone pressed
the [submit] button for merging, and before the actual commit is pushed
into the branch. If there is no config option, such a hook-script could
be made to work. But, my Gerrit experience on that level is
non-existent, so I can be completely wrong.

Niels

> 
> 
> >
> > I like that one - thanks! :-)
> >
> > Michael
> >
> > > - Original Message -
> > > > On 2016-09-30 at 17:52 +0200, Niels de Vos wrote:
> > > > > On Fri, Sep 30, 2016 at 08:50:12PM +0530, Ravishankar N wrote:
> > > > > > On 09/30/2016 06:38 PM, Niels de Vos wrote:
> > > > > > > On Fri, Sep 30, 2016 at 07:11:51AM +0530, Pranith Kumar Karampuri
> > > > > > > wrote:
> > > > > ...
> > > > > > > Maybe we can add an additional tag that mentions all the people
> > that
> > > > > > > did do reviews of older versions of the patch. Not sure what the
> > tag
> > > > > > > would be, maybe just CC?
> > > > > > It depends on what tags would be processed to obtain statistics on
> > review
> > > > > > contributions.
> > > > >
> > > > > Real statistics would come from Gerrit, not from the 'git log'
> > output.
> > > > > We do have a ./extras/who-wrote-glusterfs/ in the sources, but that
> > is
> > > > > only to get an idea about the changes that were made and should not
> > be
> > > > > used for serious statistics.
> > > > >
> > > > > It is possible to feed the Gerrit comment-stream into things like
> > > > > Elasticsearch and get an accurate impression how many reviews people
> > do
> > > > > (and much more). I hope we can get some contribution diagrams from
> > > > > someting like this at one point.
> > > > >
> > > > > Would some kind of Gave-feedback tag for people that left a comment
> > on
> > > > > earlier versions of the patch be appreciated by others? It will show
> > in
> > > > > the 'git log' who was involved in some way or form.
> > > >
> > > > I think this would be fair.
> > > >
> > > > Reviewed-by tags should imho be reserved for the final
> > > > incarnation of the patch. Those mean that the person named
> > > > in the tag has aproved this version of the patch for getting
> > > > into the official tree. A previous version of the patch can
> > > > have been entirely different, so a reviewed-by for that
> > > > previous version may not actually apply to the new version at all
> > > > and hence create a false impression!
> > > >
> > > > It is also difficult to track all activities by tags,
> > > > and anyone who wants to measure performance and contributions
> > > > only by looking at git commit tags will not be doing several
> > > > people justice. We could add 'discussed-with' or 'designed-by'
> > > > tags, etc ... ;-)
> > > >
> > > > On a serious note, in Samba we use 'Pair-programmed-with' tags,
> > > > because we do pair-programming a lot, but only one person can
> > > > be an author of a git commit ...
> > > >
> > > > The 'Gave-feedback' tag I do like. even though it does
> > > > not quite match with the foobar-by pattern of other tags.
> > > >
> > > > Michael
> > > >
> > > > ___
> > > > Gluster-devel mailing list
> > > > gluster-de...@gluster.org
> > > > http://www.gluster.org/mailman/listinfo/gluster-devel
> >
> > ___
> > maintainers mailing list
> > maintainers@gluster.org
> > http://www.gluster.org/mailman/listinfo/maintainers
> >
> >
> 
> 
> -- 
> Pranith

> ___
> maintainers mailing list
> maintainers@gluster.org
> http://www.gluster.org/mailman/listinfo/maintainers



signature.asc
Description: PGP signature
___
maintainers mailing list
maintainers@gluster.org
http://www.gluster.org/mailman/listinfo/maintainers

Re: [Gluster-Maintainers] [Gluster-devel] 'Reviewd-by' tag for commits

[Michael Adam]

On 2016-10-05 at 09:45 -0400, Ira Cooper wrote:
> "Feedback-given-by: "

I like that one - thanks! :-)

Michael

> - Original Message -
> > On 2016-09-30 at 17:52 +0200, Niels de Vos wrote:
> > > On Fri, Sep 30, 2016 at 08:50:12PM +0530, Ravishankar N wrote:
> > > > On 09/30/2016 06:38 PM, Niels de Vos wrote:
> > > > > On Fri, Sep 30, 2016 at 07:11:51AM +0530, Pranith Kumar Karampuri
> > > > > wrote:
> > > ...
> > > > > Maybe we can add an additional tag that mentions all the people that
> > > > > did do reviews of older versions of the patch. Not sure what the tag
> > > > > would be, maybe just CC?
> > > > It depends on what tags would be processed to obtain statistics on 
> > > > review
> > > > contributions.
> > > 
> > > Real statistics would come from Gerrit, not from the 'git log' output.
> > > We do have a ./extras/who-wrote-glusterfs/ in the sources, but that is
> > > only to get an idea about the changes that were made and should not be
> > > used for serious statistics.
> > > 
> > > It is possible to feed the Gerrit comment-stream into things like
> > > Elasticsearch and get an accurate impression how many reviews people do
> > > (and much more). I hope we can get some contribution diagrams from
> > > someting like this at one point.
> > > 
> > > Would some kind of Gave-feedback tag for people that left a comment on
> > > earlier versions of the patch be appreciated by others? It will show in
> > > the 'git log' who was involved in some way or form.
> > 
> > I think this would be fair.
> > 
> > Reviewed-by tags should imho be reserved for the final
> > incarnation of the patch. Those mean that the person named
> > in the tag has aproved this version of the patch for getting
> > into the official tree. A previous version of the patch can
> > have been entirely different, so a reviewed-by for that
> > previous version may not actually apply to the new version at all
> > and hence create a false impression!
> > 
> > It is also difficult to track all activities by tags,
> > and anyone who wants to measure performance and contributions
> > only by looking at git commit tags will not be doing several
> > people justice. We could add 'discussed-with' or 'designed-by'
> > tags, etc ... ;-)
> > 
> > On a serious note, in Samba we use 'Pair-programmed-with' tags,
> > because we do pair-programming a lot, but only one person can
> > be an author of a git commit ...
> > 
> > The 'Gave-feedback' tag I do like. even though it does
> > not quite match with the foobar-by pattern of other tags.
> > 
> > Michael
> > 
> > ___
> > Gluster-devel mailing list
> > gluster-de...@gluster.org
> > http://www.gluster.org/mailman/listinfo/gluster-devel


signature.asc
Description: PGP signature
___
maintainers mailing list
maintainers@gluster.org
http://www.gluster.org/mailman/listinfo/maintainers

Re: [Gluster-Maintainers] [Gluster-devel] 'Reviewd-by' tag for commits

[Ira Cooper]

"Feedback-given-by: "

Cheers,

-IRa

- Original Message -
> On 2016-09-30 at 17:52 +0200, Niels de Vos wrote:
> > On Fri, Sep 30, 2016 at 08:50:12PM +0530, Ravishankar N wrote:
> > > On 09/30/2016 06:38 PM, Niels de Vos wrote:
> > > > On Fri, Sep 30, 2016 at 07:11:51AM +0530, Pranith Kumar Karampuri
> > > > wrote:
> > ...
> > > > Maybe we can add an additional tag that mentions all the people that
> > > > did do reviews of older versions of the patch. Not sure what the tag
> > > > would be, maybe just CC?
> > > It depends on what tags would be processed to obtain statistics on review
> > > contributions.
> > 
> > Real statistics would come from Gerrit, not from the 'git log' output.
> > We do have a ./extras/who-wrote-glusterfs/ in the sources, but that is
> > only to get an idea about the changes that were made and should not be
> > used for serious statistics.
> > 
> > It is possible to feed the Gerrit comment-stream into things like
> > Elasticsearch and get an accurate impression how many reviews people do
> > (and much more). I hope we can get some contribution diagrams from
> > someting like this at one point.
> > 
> > Would some kind of Gave-feedback tag for people that left a comment on
> > earlier versions of the patch be appreciated by others? It will show in
> > the 'git log' who was involved in some way or form.
> 
> I think this would be fair.
> 
> Reviewed-by tags should imho be reserved for the final
> incarnation of the patch. Those mean that the person named
> in the tag has aproved this version of the patch for getting
> into the official tree. A previous version of the patch can
> have been entirely different, so a reviewed-by for that
> previous version may not actually apply to the new version at all
> and hence create a false impression!
> 
> It is also difficult to track all activities by tags,
> and anyone who wants to measure performance and contributions
> only by looking at git commit tags will not be doing several
> people justice. We could add 'discussed-with' or 'designed-by'
> tags, etc ... ;-)
> 
> On a serious note, in Samba we use 'Pair-programmed-with' tags,
> because we do pair-programming a lot, but only one person can
> be an author of a git commit ...
> 
> The 'Gave-feedback' tag I do like. even though it does
> not quite match with the foobar-by pattern of other tags.
> 
> Michael
> 
> ___
> Gluster-devel mailing list
> gluster-de...@gluster.org
> http://www.gluster.org/mailman/listinfo/gluster-devel
___
maintainers mailing list
maintainers@gluster.org
http://www.gluster.org/mailman/listinfo/maintainers

Re: [Gluster-Maintainers] [Gluster-devel] 'Reviewd-by' tag for commits

[Michael Adam]

On 2016-09-30 at 17:52 +0200, Niels de Vos wrote:
> On Fri, Sep 30, 2016 at 08:50:12PM +0530, Ravishankar N wrote:
> > On 09/30/2016 06:38 PM, Niels de Vos wrote:
> > > On Fri, Sep 30, 2016 at 07:11:51AM +0530, Pranith Kumar Karampuri wrote:
> ...
> > > Maybe we can add an additional tag that mentions all the people that
> > > did do reviews of older versions of the patch. Not sure what the tag
> > > would be, maybe just CC?
> > It depends on what tags would be processed to obtain statistics on review
> > contributions.
> 
> Real statistics would come from Gerrit, not from the 'git log' output.
> We do have a ./extras/who-wrote-glusterfs/ in the sources, but that is
> only to get an idea about the changes that were made and should not be
> used for serious statistics.
> 
> It is possible to feed the Gerrit comment-stream into things like
> Elasticsearch and get an accurate impression how many reviews people do
> (and much more). I hope we can get some contribution diagrams from
> someting like this at one point.
> 
> Would some kind of Gave-feedback tag for people that left a comment on
> earlier versions of the patch be appreciated by others? It will show in
> the 'git log' who was involved in some way or form.

I think this would be fair.

Reviewed-by tags should imho be reserved for the final
incarnation of the patch. Those mean that the person named
in the tag has aproved this version of the patch for getting
into the official tree. A previous version of the patch can
have been entirely different, so a reviewed-by for that
previous version may not actually apply to the new version at all
and hence create a false impression!

It is also difficult to track all activities by tags,
and anyone who wants to measure performance and contributions
only by looking at git commit tags will not be doing several
people justice. We could add 'discussed-with' or 'designed-by'
tags, etc ... ;-)

On a serious note, in Samba we use 'Pair-programmed-with' tags,
because we do pair-programming a lot, but only one person can
be an author of a git commit ...

The 'Gave-feedback' tag I do like. even though it does
not quite match with the foobar-by pattern of other tags.

Michael


signature.asc
Description: PGP signature
___
maintainers mailing list
maintainers@gluster.org
http://www.gluster.org/mailman/listinfo/maintainers

Re: [Gluster-Maintainers] [Gluster-devel] 'Reviewd-by' tag for commits

[Ira Cooper]

"Feedback-given-by: "

Cheers,

-Ira

- Original Message -
> On 2016-09-30 at 17:52 +0200, Niels de Vos wrote:
> > On Fri, Sep 30, 2016 at 08:50:12PM +0530, Ravishankar N wrote:
> > > On 09/30/2016 06:38 PM, Niels de Vos wrote:
> > > > On Fri, Sep 30, 2016 at 07:11:51AM +0530, Pranith Kumar Karampuri
> > > > wrote:
> > ...
> > > > Maybe we can add an additional tag that mentions all the people that
> > > > did do reviews of older versions of the patch. Not sure what the tag
> > > > would be, maybe just CC?
> > > It depends on what tags would be processed to obtain statistics on review
> > > contributions.
> > 
> > Real statistics would come from Gerrit, not from the 'git log' output.
> > We do have a ./extras/who-wrote-glusterfs/ in the sources, but that is
> > only to get an idea about the changes that were made and should not be
> > used for serious statistics.
> > 
> > It is possible to feed the Gerrit comment-stream into things like
> > Elasticsearch and get an accurate impression how many reviews people do
> > (and much more). I hope we can get some contribution diagrams from
> > someting like this at one point.
> > 
> > Would some kind of Gave-feedback tag for people that left a comment on
> > earlier versions of the patch be appreciated by others? It will show in
> > the 'git log' who was involved in some way or form.
> 
> I think this would be fair.
> 
> Reviewed-by tags should imho be reserved for the final
> incarnation of the patch. Those mean that the person named
> in the tag has aproved this version of the patch for getting
> into the official tree. A previous version of the patch can
> have been entirely different, so a reviewed-by for that
> previous version may not actually apply to the new version at all
> and hence create a false impression!
> 
> It is also difficult to track all activities by tags,
> and anyone who wants to measure performance and contributions
> only by looking at git commit tags will not be doing several
> people justice. We could add 'discussed-with' or 'designed-by'
> tags, etc ... ;-)
> 
> On a serious note, in Samba we use 'Pair-programmed-with' tags,
> because we do pair-programming a lot, but only one person can
> be an author of a git commit ...
> 
> The 'Gave-feedback' tag I do like. even though it does
> not quite match with the foobar-by pattern of other tags.
> 
> Michael
> 
> ___
> Gluster-devel mailing list
> gluster-de...@gluster.org
> http://www.gluster.org/mailman/listinfo/gluster-devel
___
maintainers mailing list
maintainers@gluster.org
http://www.gluster.org/mailman/listinfo/maintainers

Re: [Gluster-Maintainers] [Gluster-devel] 'Reviewd-by' tag for commits

[Pranith Kumar Karampuri]

On Mon, Oct 3, 2016 at 12:17 PM, Joe Julian  wrote:

> If you get credit for +1, shouldn't you also get credit for -1? It seems
> to me that catching a fault is at least as valuable if not more so.
>

Yes when I said review it could be either +1/-1/+2


>
> On October 3, 2016 3:58:32 AM GMT+02:00, Pranith Kumar Karampuri <
> pkara...@redhat.com> wrote:
>>
>>
>>
>> On Mon, Oct 3, 2016 at 7:23 AM, Ravishankar N 
>> wrote:
>>
>>> On 10/03/2016 06:58 AM, Pranith Kumar Karampuri wrote:
>>>
>>>
>>>
>>> On Mon, Oct 3, 2016 at 6:41 AM, Pranith Kumar Karampuri <
>>> pkara...@redhat.com> wrote:
>>>


 On Fri, Sep 30, 2016 at 8:50 PM, Ravishankar N 
 wrote:

> On 09/30/2016 06:38 PM, Niels de Vos wrote:
>
> On Fri, Sep 30, 2016 at 07:11:51AM +0530, Pranith Kumar Karampuri wrote:
>
> hi,
>  At the moment 'Reviewed-by' tag comes only if a +1 is given on the
> final version of the patch. But for most of the patches, different people
> would spend time on different versions making the patch better, they may
> not get time to do the review for every version of the patch. Is it
> possible to change the gerrit script to add 'Reviewed-by' for all the
> people who participated in the review?
>
> +1 to this. For the argument that this *might* encourage me-too +1s,
> it only exposes
> such persons in bad light.
>
> Or removing 'Reviewed-by' tag completely would also help to make sure it
> doesn't give skewed counts.
>
> I'm not going to lie, for me, that takes away the incentive of doing
> any reviews at all.
>

 Could you elaborate why? May be you should also talk about your primary
 motivation for doing reviews.

>>>
>>> I guess it is probably because the effort needs to be recognized? I
>>> think there is an option to recognize it so it is probably not a good idea
>>> to remove the tag I guess.
>>>
>>>
>>> Yes, numbers provide good motivation for me:
>>> Motivation for looking at patches and finding bugs for known components
>>> even though I am not its maintainer.
>>> Motivation to learning new components because a bug and a fix is usually
>>> when I look at code for unknown components.
>>> Motivation to level-up when statistics indicate I'm behind my peers.
>>>
>>> I think even you said some time back in an ML thread that what can be
>>> measured can be improved.
>>>
>>
>> I am still not sure how to quantify good review from a bad one. So not
>> sure how it can be measured thus improved. I guess at this point getting
>> more eyes on the patches is good enough.
>>
>>
>>>
>>> -Ravi
>>>
>>>
>>>

 I would not feel comfortable automatically adding Reviewed-by tags for
> people that did not review the last version. They may not agree with the
> last version, so adding their "approved stamp" on it may not be correct.
> See the description of Reviewed-by in the Linux kernel sources [0].
>
> While the Linux kernel model is the poster child for projects to draw
> standards
> from, IMO, their email based review system is certainly not one to
> emulate. It
> does not provide a clean way to view patch-set diffs, does not present
> a single
> URL based history that tracks all review comments, relies on the
> sender to
> provide information on what changed between versions, allows a variety
> of
> 'Komedians' [1] to add random tags which may or may not be picked up
> by the maintainer who takes patches in etc.
>
> Maybe we can add an additional tag that mentions all the people that
> did do reviews of older versions of the patch. Not sure what the tag
> would be, maybe just CC?
>
> It depends on what tags would be processed to obtain statistics on
> review contributions.
> I agree that not all reviewers might be okay with the latest revision
> but that
> % might be miniscule (zero, really) compared to the normal case where
> the reviewer spent
> considerable time and effort to provide feedback (and an eventual +1)
> on previous
> revisions. If converting all +1s into 'Reviewed-by's is not feasible
> in gerrit
> or is not considered acceptable, then the maintainer could wait for a
> reasonable
> time for reviewers to give +1 for the final revision before he/she
> goes ahead
> with a +2 and merges it. While we cannot wait indefinitely for all
> acks, a comment
> like 'LGTM, will wait for a day for other acks before I go ahead and
> merge' would be
> appreciated.
>
> Enough of bike-shedding from my end I suppose.:-)
> Ravi
>
> [1] https://lwn.net/Articles/503829/
>
> Niels
>
> 0. 
> http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/SubmittingPatches#n552
>
> 

Re: [Gluster-Maintainers] [Gluster-devel] 'Reviewd-by' tag for commits

[Pranith Kumar Karampuri]

On Mon, Oct 3, 2016 at 7:23 AM, Ravishankar N 
wrote:

> On 10/03/2016 06:58 AM, Pranith Kumar Karampuri wrote:
>
>
>
> On Mon, Oct 3, 2016 at 6:41 AM, Pranith Kumar Karampuri <
> pkara...@redhat.com> wrote:
>
>>
>>
>> On Fri, Sep 30, 2016 at 8:50 PM, Ravishankar N 
>> wrote:
>>
>>> On 09/30/2016 06:38 PM, Niels de Vos wrote:
>>>
>>> On Fri, Sep 30, 2016 at 07:11:51AM +0530, Pranith Kumar Karampuri wrote:
>>>
>>> hi,
>>>  At the moment 'Reviewed-by' tag comes only if a +1 is given on the
>>> final version of the patch. But for most of the patches, different people
>>> would spend time on different versions making the patch better, they may
>>> not get time to do the review for every version of the patch. Is it
>>> possible to change the gerrit script to add 'Reviewed-by' for all the
>>> people who participated in the review?
>>>
>>> +1 to this. For the argument that this *might* encourage me-too +1s, it
>>> only exposes
>>> such persons in bad light.
>>>
>>> Or removing 'Reviewed-by' tag completely would also help to make sure it
>>> doesn't give skewed counts.
>>>
>>> I'm not going to lie, for me, that takes away the incentive of doing any
>>> reviews at all.
>>>
>>
>> Could you elaborate why? May be you should also talk about your primary
>> motivation for doing reviews.
>>
>
> I guess it is probably because the effort needs to be recognized? I think
> there is an option to recognize it so it is probably not a good idea to
> remove the tag I guess.
>
>
> Yes, numbers provide good motivation for me:
> Motivation for looking at patches and finding bugs for known components
> even though I am not its maintainer.
> Motivation to learning new components because a bug and a fix is usually
> when I look at code for unknown components.
> Motivation to level-up when statistics indicate I'm behind my peers.
>
> I think even you said some time back in an ML thread that what can be
> measured can be improved.
>

I am still not sure how to quantify good review from a bad one. So not sure
how it can be measured thus improved. I guess at this point getting more
eyes on the patches is good enough.


>
> -Ravi
>
>
>
>>
>> I would not feel comfortable automatically adding Reviewed-by tags for
>>> people that did not review the last version. They may not agree with the
>>> last version, so adding their "approved stamp" on it may not be correct.
>>> See the description of Reviewed-by in the Linux kernel sources [0].
>>>
>>> While the Linux kernel model is the poster child for projects to draw
>>> standards
>>> from, IMO, their email based review system is certainly not one to
>>> emulate. It
>>> does not provide a clean way to view patch-set diffs, does not present a
>>> single
>>> URL based history that tracks all review comments, relies on the sender
>>> to
>>> provide information on what changed between versions, allows a variety of
>>> 'Komedians' [1] to add random tags which may or may not be picked up
>>> by the maintainer who takes patches in etc.
>>>
>>> Maybe we can add an additional tag that mentions all the people that
>>> did do reviews of older versions of the patch. Not sure what the tag
>>> would be, maybe just CC?
>>>
>>> It depends on what tags would be processed to obtain statistics on
>>> review contributions.
>>> I agree that not all reviewers might be okay with the latest revision
>>> but that
>>> % might be miniscule (zero, really) compared to the normal case where
>>> the reviewer spent
>>> considerable time and effort to provide feedback (and an eventual +1) on
>>> previous
>>> revisions. If converting all +1s into 'Reviewed-by's is not feasible in
>>> gerrit
>>> or is not considered acceptable, then the maintainer could wait for a
>>> reasonable
>>> time for reviewers to give +1 for the final revision before he/she goes
>>> ahead
>>> with a +2 and merges it. While we cannot wait indefinitely for all acks,
>>> a comment
>>> like 'LGTM, will wait for a day for other acks before I go ahead and
>>> merge' would be
>>> appreciated.
>>>
>>> Enough of bike-shedding from my end I suppose.:-)
>>> Ravi
>>>
>>> [1] https://lwn.net/Articles/503829/
>>>
>>> Niels
>>>
>>> 0. 
>>> http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/SubmittingPatches#n552
>>>
>>> ___
>>> Gluster-devel mailing 
>>> listGluster-devel@gluster.orghttp://www.gluster.org/mailman/listinfo/gluster-devel
>>>
>>> --
>> Pranith
>>
> --
> Pranith
>
>


-- 
Pranith
___
maintainers mailing list
maintainers@gluster.org
http://www.gluster.org/mailman/listinfo/maintainers

Re: [Gluster-Maintainers] [Gluster-devel] 'Reviewd-by' tag for commits

[Ravishankar N]

On 10/03/2016 06:58 AM, Pranith Kumar Karampuri wrote:



On Mon, Oct 3, 2016 at 6:41 AM, Pranith Kumar Karampuri 
> wrote:




On Fri, Sep 30, 2016 at 8:50 PM, Ravishankar N
> wrote:

On 09/30/2016 06:38 PM, Niels de Vos wrote:

On Fri, Sep 30, 2016 at 07:11:51AM +0530, Pranith Kumar Karampuri wrote:

hi,
  At the moment 'Reviewed-by' tag comes only if a +1 is given on the
final version of the patch. But for most of the patches, different 
people
would spend time on different versions making the patch better, they may
not get time to do the review for every version of the patch. Is it
possible to change the gerrit script to add 'Reviewed-by' for all the
people who participated in the review?

+1 to this. For the argument that this *might* encourage
me-too +1s, it only exposes
such persons in bad light.

Or removing 'Reviewed-by' tag completely would also help to make sure it
doesn't give skewed counts.

I'm not going to lie, for me, that takes away the incentive of
doing any reviews at all.


Could you elaborate why? May be you should also talk about your
primary motivation for doing reviews.


I guess it is probably because the effort needs to be recognized? I 
think there is an option to recognize it so it is probably not a good 
idea to remove the tag I guess.


Yes, numbers provide good motivation for me:
Motivation for looking at patches and finding bugs for known components 
even though I am not its maintainer.
Motivation to learning new components because a bug and a fix is usually 
when I look at code for unknown components.

Motivation to level-up when statistics indicate I'm behind my peers.

I think even you said some time back in an ML thread that what can be 
measured can be improved.


-Ravi




I would not feel comfortable automatically adding Reviewed-by tags for
people that did not review the last version. They may not agree with the
last version, so adding their "approved stamp" on it may not be correct.
See the description of Reviewed-by in the Linux kernel sources [0].

While the Linux kernel model is the poster child for projects
to draw standards
from, IMO, their email based review system is certainly not
one to emulate. It
does not provide a clean way to view patch-set diffs, does not
present a single
URL based history that tracks all review comments, relies on
the sender to
provide information on what changed between versions, allows a
variety of
'Komedians' [1] to add random tags which may or may not be
picked up
by the maintainer who takes patches in etc.

Maybe we can add an additional tag that mentions all the people that
did do reviews of older versions of the patch. Not sure what the tag
would be, maybe just CC?

It depends on what tags would be processed to obtain
statistics on review contributions.
I agree that not all reviewers might be okay with the latest
revision but that
% might be miniscule (zero, really) compared to the normal
case where the reviewer spent
considerable time and effort to provide feedback (and an
eventual +1) on previous
revisions. If converting all +1s into 'Reviewed-by's is not
feasible in gerrit
or is not considered acceptable, then the maintainer could
wait for a reasonable
time for reviewers to give +1 for the final revision before
he/she goes ahead
with a +2 and merges it. While we cannot wait indefinitely for
all acks, a comment
like 'LGTM, will wait for a day for other acks before I go
ahead and merge' would be
appreciated.

Enough of bike-shedding from my end I suppose.:-)
Ravi

[1] https://lwn.net/Articles/503829/



Niels


0.http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/SubmittingPatches#n552



___
Gluster-devel mailing list
gluster-de...@gluster.org 
http://www.gluster.org/mailman/listinfo/gluster-devel



-- 
Pranith


--
Pranith


___
maintainers mailing list
maintainers@gluster.org
http://www.gluster.org/mailman/listinfo/maintainers

Re: [Gluster-Maintainers] [Gluster-devel] 'Reviewd-by' tag for commits

[Pranith Kumar Karampuri]

On Mon, Oct 3, 2016 at 6:41 AM, Pranith Kumar Karampuri  wrote:

>
>
> On Fri, Sep 30, 2016 at 8:50 PM, Ravishankar N 
> wrote:
>
>> On 09/30/2016 06:38 PM, Niels de Vos wrote:
>>
>> On Fri, Sep 30, 2016 at 07:11:51AM +0530, Pranith Kumar Karampuri wrote:
>>
>> hi,
>>  At the moment 'Reviewed-by' tag comes only if a +1 is given on the
>> final version of the patch. But for most of the patches, different people
>> would spend time on different versions making the patch better, they may
>> not get time to do the review for every version of the patch. Is it
>> possible to change the gerrit script to add 'Reviewed-by' for all the
>> people who participated in the review?
>>
>> +1 to this. For the argument that this *might* encourage me-too +1s, it
>> only exposes
>> such persons in bad light.
>>
>> Or removing 'Reviewed-by' tag completely would also help to make sure it
>> doesn't give skewed counts.
>>
>> I'm not going to lie, for me, that takes away the incentive of doing any
>> reviews at all.
>>
>
> Could you elaborate why? May be you should also talk about your primary
> motivation for doing reviews.
>

I guess it is probably because the effort needs to be recognized? I think
there is an option to recognize it so it is probably not a good idea to
remove the tag I guess.


>
> I would not feel comfortable automatically adding Reviewed-by tags for
>> people that did not review the last version. They may not agree with the
>> last version, so adding their "approved stamp" on it may not be correct.
>> See the description of Reviewed-by in the Linux kernel sources [0].
>>
>> While the Linux kernel model is the poster child for projects to draw
>> standards
>> from, IMO, their email based review system is certainly not one to
>> emulate. It
>> does not provide a clean way to view patch-set diffs, does not present a
>> single
>> URL based history that tracks all review comments, relies on the sender to
>> provide information on what changed between versions, allows a variety of
>> 'Komedians' [1] to add random tags which may or may not be picked up
>> by the maintainer who takes patches in etc.
>>
>> Maybe we can add an additional tag that mentions all the people that
>> did do reviews of older versions of the patch. Not sure what the tag
>> would be, maybe just CC?
>>
>> It depends on what tags would be processed to obtain statistics on review
>> contributions.
>> I agree that not all reviewers might be okay with the latest revision but
>> that
>> % might be miniscule (zero, really) compared to the normal case where the
>> reviewer spent
>> considerable time and effort to provide feedback (and an eventual +1) on
>> previous
>> revisions. If converting all +1s into 'Reviewed-by's is not feasible in
>> gerrit
>> or is not considered acceptable, then the maintainer could wait for a
>> reasonable
>> time for reviewers to give +1 for the final revision before he/she goes
>> ahead
>> with a +2 and merges it. While we cannot wait indefinitely for all acks,
>> a comment
>> like 'LGTM, will wait for a day for other acks before I go ahead and
>> merge' would be
>> appreciated.
>>
>> Enough of bike-shedding from my end I suppose.:-)
>> Ravi
>>
>> [1] https://lwn.net/Articles/503829/
>>
>> Niels
>>
>> 0. 
>> http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/SubmittingPatches#n552
>>
>>
>>
>> ___
>> Gluster-devel mailing 
>> listGluster-devel@gluster.orghttp://www.gluster.org/mailman/listinfo/gluster-devel
>>
>>
>>
>
>
> --
> Pranith
>



-- 
Pranith
___
maintainers mailing list
maintainers@gluster.org
http://www.gluster.org/mailman/listinfo/maintainers

Re: [Gluster-Maintainers] [Gluster-devel] 'Reviewd-by' tag for commits

[Pranith Kumar Karampuri]

On Fri, Sep 30, 2016 at 8:50 PM, Ravishankar N 
wrote:

> On 09/30/2016 06:38 PM, Niels de Vos wrote:
>
> On Fri, Sep 30, 2016 at 07:11:51AM +0530, Pranith Kumar Karampuri wrote:
>
> hi,
>  At the moment 'Reviewed-by' tag comes only if a +1 is given on the
> final version of the patch. But for most of the patches, different people
> would spend time on different versions making the patch better, they may
> not get time to do the review for every version of the patch. Is it
> possible to change the gerrit script to add 'Reviewed-by' for all the
> people who participated in the review?
>
> +1 to this. For the argument that this *might* encourage me-too +1s, it
> only exposes
> such persons in bad light.
>
> Or removing 'Reviewed-by' tag completely would also help to make sure it
> doesn't give skewed counts.
>
> I'm not going to lie, for me, that takes away the incentive of doing any
> reviews at all.
>

Could you elaborate why? May be you should also talk about your primary
motivation for doing reviews.

I would not feel comfortable automatically adding Reviewed-by tags for
> people that did not review the last version. They may not agree with the
> last version, so adding their "approved stamp" on it may not be correct.
> See the description of Reviewed-by in the Linux kernel sources [0].
>
> While the Linux kernel model is the poster child for projects to draw
> standards
> from, IMO, their email based review system is certainly not one to
> emulate. It
> does not provide a clean way to view patch-set diffs, does not present a
> single
> URL based history that tracks all review comments, relies on the sender to
> provide information on what changed between versions, allows a variety of
> 'Komedians' [1] to add random tags which may or may not be picked up
> by the maintainer who takes patches in etc.
>
> Maybe we can add an additional tag that mentions all the people that
> did do reviews of older versions of the patch. Not sure what the tag
> would be, maybe just CC?
>
> It depends on what tags would be processed to obtain statistics on review
> contributions.
> I agree that not all reviewers might be okay with the latest revision but
> that
> % might be miniscule (zero, really) compared to the normal case where the
> reviewer spent
> considerable time and effort to provide feedback (and an eventual +1) on
> previous
> revisions. If converting all +1s into 'Reviewed-by's is not feasible in
> gerrit
> or is not considered acceptable, then the maintainer could wait for a
> reasonable
> time for reviewers to give +1 for the final revision before he/she goes
> ahead
> with a +2 and merges it. While we cannot wait indefinitely for all acks, a
> comment
> like 'LGTM, will wait for a day for other acks before I go ahead and
> merge' would be
> appreciated.
>
> Enough of bike-shedding from my end I suppose.:-)
> Ravi
>
> [1] https://lwn.net/Articles/503829/
>
> Niels
>
> 0. 
> http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/SubmittingPatches#n552
>
>
>
> ___
> Gluster-devel mailing 
> listGluster-devel@gluster.orghttp://www.gluster.org/mailman/listinfo/gluster-devel
>
>
>


-- 
Pranith
___
maintainers mailing list
maintainers@gluster.org
http://www.gluster.org/mailman/listinfo/maintainers

Re: [Gluster-Maintainers] [Gluster-devel] 'Reviewd-by' tag for commits

[Niels de Vos]

On Fri, Sep 30, 2016 at 08:50:12PM +0530, Ravishankar N wrote:
> On 09/30/2016 06:38 PM, Niels de Vos wrote:
> > On Fri, Sep 30, 2016 at 07:11:51AM +0530, Pranith Kumar Karampuri wrote:
...
> > Maybe we can add an additional tag that mentions all the people that
> > did do reviews of older versions of the patch. Not sure what the tag
> > would be, maybe just CC?
> It depends on what tags would be processed to obtain statistics on review
> contributions.

Real statistics would come from Gerrit, not from the 'git log' output.
We do have a ./extras/who-wrote-glusterfs/ in the sources, but that is
only to get an idea about the changes that were made and should not be
used for serious statistics.

It is possible to feed the Gerrit comment-stream into things like
Elasticsearch and get an accurate impression how many reviews people do
(and much more). I hope we can get some contribution diagrams from
someting like this at one point.

Would some kind of Gave-feedback tag for people that left a comment on
earlier versions of the patch be appreciated by others? It will show in
the 'git log' who was involved in some way or form.

Do others think this would be useful?
Niels


signature.asc
Description: PGP signature
___
maintainers mailing list
maintainers@gluster.org
http://www.gluster.org/mailman/listinfo/maintainers

Re: [Gluster-Maintainers] [Gluster-devel] 'Reviewd-by' tag for commits

[Ravishankar N]

On 09/30/2016 06:38 PM, Niels de Vos wrote:

On Fri, Sep 30, 2016 at 07:11:51AM +0530, Pranith Kumar Karampuri wrote:

hi,
  At the moment 'Reviewed-by' tag comes only if a +1 is given on the
final version of the patch. But for most of the patches, different people
would spend time on different versions making the patch better, they may
not get time to do the review for every version of the patch. Is it
possible to change the gerrit script to add 'Reviewed-by' for all the
people who participated in the review?
+1 to this. For the argument that this *might* encourage me-too +1s, it 
only exposes

such persons in bad light.

Or removing 'Reviewed-by' tag completely would also help to make sure it
doesn't give skewed counts.
I'm not going to lie, for me, that takes away the incentive of doing any 
reviews at all.

I would not feel comfortable automatically adding Reviewed-by tags for
people that did not review the last version. They may not agree with the
last version, so adding their "approved stamp" on it may not be correct.
See the description of Reviewed-by in the Linux kernel sources [0].
While the Linux kernel model is the poster child for projects to draw 
standards
from, IMO, their email based review system is certainly not one to 
emulate. It
does not provide a clean way to view patch-set diffs, does not present a 
single

URL based history that tracks all review comments, relies on the sender to
provide information on what changed between versions, allows a variety of
'Komedians' [1] to add random tags which may or may not be picked up
by the maintainer who takes patches in etc.

Maybe we can add an additional tag that mentions all the people that
did do reviews of older versions of the patch. Not sure what the tag
would be, maybe just CC?
It depends on what tags would be processed to obtain statistics on 
review contributions.
I agree that not all reviewers might be okay with the latest revision 
but that
% might be miniscule (zero, really) compared to the normal case where 
the reviewer spent
considerable time and effort to provide feedback (and an eventual +1) on 
previous
revisions. If converting all +1s into 'Reviewed-by's is not feasible in 
gerrit
or is not considered acceptable, then the maintainer could wait for a 
reasonable
time for reviewers to give +1 for the final revision before he/she goes 
ahead
with a +2 and merges it. While we cannot wait indefinitely for all acks, 
a comment
like 'LGTM, will wait for a day for other acks before I go ahead and 
merge' would be

appreciated.

Enough of bike-shedding from my end I suppose.:-)
Ravi

[1] https://lwn.net/Articles/503829/



Niels

0. 
http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/SubmittingPatches#n552


___
Gluster-devel mailing list
gluster-de...@gluster.org
http://www.gluster.org/mailman/listinfo/gluster-devel



___
maintainers mailing list
maintainers@gluster.org
http://www.gluster.org/mailman/listinfo/maintainers