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

Sat Oct 15 15:08:17 UTC 2016

On Fri, Oct 14, 2016 at 4:44 AM, Niels de Vos <ndevos at redhat.com> 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 <ndevos at redhat.com> 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 <obnox at samba.org> wrote:
>> > >
>> > > > On 2016-10-05 at 09:45 -0400, Ira Cooper wrote:
>> > > > > "Feedback-given-by: <nosy.person at silly.place>"
>> > > >
>> > >
>> > > 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-devel at gluster.org
>> > > > > > http://www.gluster.org/mailman/listinfo/gluster-devel
>> > > >
>> > > > _______________________________________________
>> > > > maintainers mailing list
>> > > > maintainers at gluster.org
>> > > > http://www.gluster.org/mailman/listinfo/maintainers
>> > > >
>> > > >
>> > >
>> > >
>> > > --
>> > > Pranith
>> >
>> > > _______________________________________________
>> > > maintainers mailing list
>> > > maintainers at gluster.org
>> > > http://www.gluster.org/mailman/listinfo/maintainers
>> >
>> >
>> > _______________________________________________
>> > maintainers mailing list
>> > maintainers at gluster.org
>> > http://www.gluster.org/mailman/listinfo/maintainers
>> >
>> >
>>
>>
>> --
>> nigelb
>
> _______________________________________________
> Gluster-devel mailing list
> Gluster-devel at gluster.org
> http://www.gluster.org/mailman/listinfo/gluster-devel