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

[Joe Julian]

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. 

On October 3, 2016 3:58:32 AM GMT+02:00, Pranith Kumar Karampuri <pkarampu at redhat.com> wrote:
>On Mon, Oct 3, 2016 at 7:23 AM, Ravishankar N <ravishankar at redhat.com>
>wrote:
>
>> 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> wrote:
>>
>>>
>>>
>>> On Fri, Sep 30, 2016 at 8:50 PM, Ravishankar N
><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:
>>>>
>>>> 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 at gluster.orghttp://www.gluster.org/mailman/listinfo/gluster-devel
>>>>
>>>> --
>>> Pranith
>>>
>> --
>> Pranith
>>
>>
>
>
>-- 
>Pranith
>
>
>------------------------------------------------------------------------
>
>_______________________________________________
>Gluster-devel mailing list
>Gluster-devel at gluster.org
>http://www.gluster.org/mailman/listinfo/gluster-devel

-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.gluster.org/pipermail/gluster-devel/attachments/20161003/64cd820b/attachment.html>