<html><head></head><body>If you get credit for +1, shouldn&#39;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 &lt;pkarampu@redhat.com&gt; 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">&lt;<a href="mailto:ravishankar@redhat.com" target="_blank">ravishankar@redhat.com</a>&gt;</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">&lt;<a href="mailto:pkarampu@redhat.com" target="_blank">pkarampu@redhat.com</a>&gt;</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">&lt;<a href="mailto:ravishankar@redhat.com" target="_blank">ravishankar@redhat.com</a>&gt;</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 &#39;Reviewed-by&#39; 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 &#39;Reviewed-by&#39; 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 &#39;Reviewed-by&#39; tag completely would also help to make sure it
doesn&#39;t give skewed counts.
</pre>
                              </blockquote>
                            </blockquote>
                          </span><tt>I&#39;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&#39;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 &quot;approved stamp&quot; 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> &#39;Komedians&#39; [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 &#39;Reviewed-by&#39;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>&#39;LGTM, will wait for a day
                              for other acks before I go ahead and
                              merge&#39; 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>