On Fri, Sep 30, 2016 at 8:50 PM, Ravishankar N <[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 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 > [email protected]http://www.gluster.org/mailman/listinfo/gluster-devel > > > -- Pranith
_______________________________________________ maintainers mailing list [email protected] http://www.gluster.org/mailman/listinfo/maintainers
