Re: How to keep Reviewed-by lines in git commits with gerrit.

[Florian Weimer <fw at deneb dot enyo dot de>]

* Carlos O'Donell:

> On 11/12/19 2:39 PM, Florian Weimer wrote:
>> * Carlos O'Donell:
>> 
>>>> We should not edit the commit message manually after it has been
>>>> reviewed.
>>>
>>> I agree. Should we setup gerrit to push to glibc git?
>> 
>> I think it's to early for that.  I'm not sure that Gerrit is the right
>> tool for us.  Too many of us have yet to use it as patch submitters
>> and reviewers.
>
> Is it too early though?
>
> Enabling gerrit to push would not stop others from pushing to master.

I see.  If we agree that it's still provisional, I think we could
activate it.  Presumably we don't have to give it a full shell account
on sourceware.org.

>> I also think that improving the email interface is a dead end and not
>> worth the effort.  Unless we are willing to abandon email-based patch
>> review, we should not switch to Gerrit (or any other web-based tool).
>
> I agree that making gerrit suit *all* of our email needs is a dead end.
>
> I do not think we will ever abandon email-based patch review.
>
> I do not think that having email-based patch review blocks our use of
> another tool for patch review.
>
> Yes, it creates 2 tools, email and another tool, as valid ways to submit
> patches, but I don't see that as a problem.

If there are problems (like approvals or objections posted to the
mailing list) I think we can make adjustments once the need arises.
I agree it's hard to predict what will happen.

>>>> The latter has the advantage that post-commit review also counts,
>>>> which is not possible if we only consider Reviewed-By: lines in
>>>> commits.
>>>
>>> Post-commit review is indeed lost in a system that uses commit
>>> messages to track review, but I'd argue that this is a such a small
>>> minority of the work that it's not worth accurately modeling in
>>> the framework.
>> 
>> I'm not sure about that.  In many cases, I've missed Reviewed-By:
>> lines due to lack of tool support.  But there have been a few where I
>> simply had not received the review email message when producing the
>> actual commit.
>
> It's hard to objectively quantify because the data is hard to get.
>
> The only thing we can do is use a system that avoids it by adding the
> reviewed-by lines automatically, like gerrit can do?
>
> Staying on email-only patch reviews we will simply continue to have
> the same issues.

Sure, if it's a stock Gerrit feature, we should enable it.

>>> The reason I'm strongly in favor of Reviwed-by: in commit messages
>>> is also that the review data goes with the commit, and clones of the
>>> repo for analysis by any other 3rd party with normal git tooling.
>> 
>> I would argue this is a fringe use case.  How is this relevant to the
>> glibc project, anyway?  It's not that we're going to promote
>> contributors due to new roles based on the number of reviews they
>> conducted.
>
> I'm not quite sure what you argue is a fringe use case? The point I made
> about 3rd parties?
>
> In general I see 3rd parties as my manager doing a review of what work
> I'm doing upstream. People not involved in the project directly.

Well yes, but that's internal Red Hat politics. 8-p

> I was initially worried that we had few reviewers, but this year
> alone we have 25 unique recorded reviewers. That's much better than
> I expected.  Next I want to see about encouraging those that
> reviewed when their reviews were fewer than 1 a month, and I thought
> it might be a good idea. Perahaps these reviewers might be
> interested in mentoring. Likewise contributors with zero recorded
> reviews might be good candidates for mentoring also.

I don't doubt the usefulness of these statistics. What I was trying to
say is that maybe the review tool is the better place to gather review
statistics.  But if Reviewed-By: gets promoted automatically and
that's what you need, then by all means use it.