[Gluster-Maintainers] [Gluster-devel] 'Reviewd-by' tag for commits
ravishankar at redhat.com
Mon Oct 3 01:53:13 UTC 2016
On 10/03/2016 06:58 AM, Pranith Kumar Karampuri wrote:
> On Mon, Oct 3, 2016 at 6:41 AM, Pranith Kumar Karampuri
> <pkarampu at redhat.com <mailto:pkarampu at redhat.com>> wrote:
> On Fri, Sep 30, 2016 at 8:50 PM, Ravishankar N
> <ravishankar at redhat.com <mailto:ravishankar at redhat.com>> 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:
>>> 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 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 .
> 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'  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
> Enough of bike-shedding from my end I suppose.:-)
>  https://lwn.net/Articles/503829/
>> Gluster-devel mailing list
>> Gluster-devel at gluster.org <mailto:Gluster-devel at gluster.org>
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the maintainers