<html><head></head><body>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. <br><br><div class="gmail_quote">On October 3, 2016 3:58:32 AM GMT+02:00, Pranith Kumar Karampuri <pkarampu@redhat.com> wrote:<blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;">
<div dir="ltr"><br /><div class="gmail_extra"><br /><div class="gmail_quote">On Mon, Oct 3, 2016 at 7:23 AM, Ravishankar N <span dir="ltr"><<a href="mailto:ravishankar@redhat.com" target="_blank">ravishankar@redhat.com</a>></span> wrote:<br /><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div bgcolor="#FFFFFF" text="#000000"><span class="">
<div>On 10/03/2016 06:58 AM, Pranith Kumar
Karampuri wrote:<br />
</div>
<blockquote type="cite">
<div dir="ltr"><br />
<div class="gmail_extra"><br />
<div class="gmail_quote">On Mon, Oct 3, 2016 at 6:41 AM,
Pranith Kumar Karampuri <span dir="ltr"><<a href="mailto:pkarampu@redhat.com" target="_blank">pkarampu@redhat.com</a>></span>
wrote:<br />
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div dir="ltr"><br />
<div class="gmail_extra"><br />
<div class="gmail_quote"><span>On Fri, Sep
30, 2016 at 8:50 PM, Ravishankar N <span dir="ltr"><<a href="mailto:ravishankar@redhat.com" target="_blank">ravishankar@redhat.com</a>></span>
wrote:<br />
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div bgcolor="#FFFFFF" text="#000000"><span>
<div>On 09/30/2016 06:38 PM, Niels de Vos
wrote:<br />
</div>
<blockquote type="cite">
<pre>On Fri, Sep 30, 2016 at 07:11:51AM +0530, Pranith Kumar Karampuri wrote:
</pre>
<blockquote type="cite">
<pre>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?</pre>
</blockquote>
</blockquote>
</span><tt>+1 to this. For the argument that
this *might* encourage me-too +1s, it only
exposes<br />
such persons in bad light.<br />
</tt><span>
<blockquote type="cite">
<blockquote type="cite">
<pre>Or removing 'Reviewed-by' tag completely would also help to make sure it
doesn't give skewed counts.
</pre>
</blockquote>
</blockquote>
</span><tt>I'm not going to lie, for me, that
takes away the incentive of doing any
reviews at all.</tt><span><br />
</span></div>
</blockquote>
<div><br />
</div>
</span>Could you elaborate why? May be you should
also talk about your primary motivation for doing
reviews.<br />
</div>
</div>
</div>
</blockquote>
<div><br />
</div>
<div>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.<br />
</div>
</div>
</div>
</div>
</blockquote>
</span><tt><br />
Yes, numbers provide good motivation for me:</tt><tt><br />
</tt><tt>Motivation for looking at patches and finding bugs for
known components even though I am not its maintainer.</tt><tt><br />
</tt><tt>Motivation to learning new components because a bug and a
fix is usually when I look at code for unknown components.</tt><tt><br />
</tt><tt>Motivation to level-up when statistics indicate I'm behind
my peers.<br />
<br />
I think even you said some time back in an ML thread that what can
be measured can be improved.<br /></tt></div></blockquote><div><br /></div><div>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.<br /></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div bgcolor="#FFFFFF" text="#000000"><tt>
<br />
-Ravi<br />
<br />
</tt><div><div class="h5">
<blockquote type="cite">
<div dir="ltr">
<div class="gmail_extra">
<div class="gmail_quote">
<div> </div>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div dir="ltr">
<div class="gmail_extra">
<div class="gmail_quote"><br />
</div>
<div>
<div>
<div class="gmail_quote">
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div bgcolor="#FFFFFF" text="#000000"><span>
<blockquote type="cite">
<pre>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].
</pre>
</blockquote>
</span><tt>While the Linux kernel model is
the poster child for projects to draw
standards</tt><tt><br />
</tt><tt> from, IMO, their email based
review system is certainly not one to
emulate. It</tt><tt><br />
</tt><tt> does not provide a clean way to
view patch-set diffs, does not present a
single</tt><tt><br />
</tt><tt> URL based history that tracks all
review comments, relies on the sender to</tt><tt><br />
</tt><tt> provide information on what
changed between versions, allows a variety
of</tt><tt><br />
</tt><tt> 'Komedians' [1] to add random tags
which may or may not be picked up</tt><tt><br />
</tt><tt> by the maintainer who takes
patches in etc.</tt><span>
<blockquote type="cite">
<pre>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?</pre>
</blockquote>
</span><tt>It depends on what tags would be
processed to obtain statistics on review
contributions.</tt><tt><br />
</tt><tt>I agree that not all reviewers
might be okay with the latest revision but
that</tt><tt><br />
</tt><tt>% might be miniscule (zero, really)
compared to the normal case where the
reviewer spent </tt><tt><br />
</tt><tt>considerable time and effort to
provide feedback (and an eventual +1) on
previous</tt><tt><br />
</tt><tt>revisions. If converting all +1s
into 'Reviewed-by's is not feasible in
gerrit</tt><tt><br />
</tt><tt>or is not considered acceptable,
then the maintainer could wait for a
reasonable</tt><tt><br />
</tt><tt>time for reviewers to give +1 for
the final revision before he/she goes
ahead</tt><tt><br />
</tt><tt>with a +2 and merges it. While we
cannot wait indefinitely for all acks, a
comment<br />
like </tt><tt>'LGTM, will wait for a day
for other acks before I go ahead and
merge' would be<br />
appreciated.<br />
<br />
Enough of bike-shedding from my end I
suppose.<span><span>:-)</span></span><br />
Ravi </tt><br />
<br />
<tt>[1] <a href="https://lwn.net/Articles/503829/" target="_blank">https://lwn.net/Articles/50382<wbr />9/</a></tt><br />
<br />
<blockquote type="cite">
<pre>Niels
0. <a href="http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/SubmittingPatches#n552" target="_blank">http://git.kernel.org/cgit/lin<wbr />ux/kernel/git/torvalds/linux.g<wbr />it/tree/Documentation/Submitti<wbr />ngPatches#n552</a>
</pre>
<fieldset></fieldset>
<pre>______________________________<wbr />_________________
Gluster-devel mailing list
<a href="mailto:Gluster-devel@gluster.org" target="_blank">Gluster-devel@gluster.org</a>
<a href="http://www.gluster.org/mailman/listinfo/gluster-devel" target="_blank">http://www.gluster.org/mailman<wbr />/listinfo/gluster-devel</a></pre>
</blockquote>
<p>
</p>
</div>
</blockquote></div>
</div></div><span><font color="#888888">--
</font><div data-smartmail="gmail_signature"><font color="#888888"></font><div dir="ltr"><font color="#888888">Pranith
</font></div></div>
</span></div></div>
</blockquote></div>
--
<div data-smartmail="gmail_signature"><div dir="ltr">Pranith
</div></div>
</div></div>
</blockquote><p>
</p></div></div></div></blockquote></div><br /><br clear="all" /></div></div></blockquote></div><br>
-- <br>
Sent from my Android device with K-9 Mail. Please excuse my brevity.</body></html>