On 10/03/2016 06:58 AM, Pranith Kumar Karampuri wrote:
On Mon, Oct 3, 2016 at 6:41 AM, Pranith Kumar Karampuri
<[email protected] <mailto:[email protected]>> wrote:
On Fri, Sep 30, 2016 at 8:50 PM, Ravishankar N
<[email protected] <mailto:[email protected]>> 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.
-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/
<https://lwn.net/Articles/503829/>
Niels
0.http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/SubmittingPatches#n552
<http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/SubmittingPatches#n552>
_______________________________________________
Gluster-devel mailing list
[email protected] <mailto:[email protected]>
http://www.gluster.org/mailman/listinfo/gluster-devel
<http://www.gluster.org/mailman/listinfo/gluster-devel>
--
Pranith
--
Pranith
_______________________________________________
maintainers mailing list
[email protected]
http://www.gluster.org/mailman/listinfo/maintainers
