This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


On 11/12/19 2:07 PM, Florian Weimer wrote:
> * Carlos O'Donell:
> 
>> If we ever switch to supporting Gerrit, then we will automatically
>> gain the Reviewed-by: lines depending on the configuration of Gerrit.
>> Reviewers would understand this as part of doing their reviews and
>> granting the review +1/+2.
>>
>> In the meantime I can do two things as a reviewer to help you keep
>> the Reviewed-by lines.
>>
>> 1. If I am about to grant +2 review I edit the commit and generate
>>    a new patchset version, and add my Reviewed-by line.
>> 2. I submit my review of +2.
>>
>> Then when you push, you just need to make sure your new commit message
>> matches and it should close the review.
> 
> We should not edit the commit message manually after it has been
> reviewed.

I agree. Should we setup gerrit to push to glibc git?

> Either it is automatically added by the review tool, or we need to
> gather patch review statistics from the review tool.

I would prefer it to be automatically added by the review tool, as
gerrit is designed to do.

> 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.

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.

-- 
Cheers,
Carlos.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]