On Mon, Oct 3, 2016 at 6:41 AM, Pranith Kumar Karampuri <pkara...@redhat.com > wrote:
> > > On Fri, Sep 30, 2016 at 8:50 PM, Ravishankar N <ravishan...@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. > > 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@gluster.orghttp://www.gluster.org/mailman/listinfo/gluster-devel >> >> >> > > > -- > Pranith > -- Pranith
_______________________________________________ Gluster-devel mailing list Gluster-devel@gluster.org http://www.gluster.org/mailman/listinfo/gluster-devel