[Freeipa-devel] Using the Reviewed-by git tag

[Martin Kosek]

On 02/10/2014 01:55 PM, Petr Viktorin wrote:
> On 02/10/2014 01:32 PM, Martin Kosek wrote:
>> Hello,
>>
>> I would like to follow up on a core devel team discussion we had last week. We
>> found out, that it would be beneficial to see a reviewer of the patches that
>> land in our git.
>>
>> This will serve both as a nice way to both generate statistics who is devoted
>> to both writing new code, but also to reviewing other people's code (and win
>> prizes ;-), but it will also offer the git history archaeologist 2 names of
>> developers which should be the most knowledgeable about the patch.
>>
>> We will use the current de-facto standard "Reviewed-By" tag. Example:
>>
>> commit da70c6d9353cd29531c8e2c135db81a97f22293c
>> Author: Martin Kosek <mkosek at redhat.com>
>> Date:   Mon Jan 27 12:28:12 2014 +0100
>>
>>      Migration does not add users to default group
>>
>>      When users with missing default group were searched, IPA suffix was
>>      not passed so these users were searched in a wrong base DN. Thus,
>>      no user was detected and added to default group.
>>
>>      https://fedorahosted.org/freeipa/ticket/4141
>>
>>      Reviewed-By: Petr Viktorin <pviktori at redhat.com>
>>
>>
>> Currently, I used to add the tag via "git commit --amend". Does anybody have a
>> nice helper scripts or snippets to semi-automate it? Note that we will be able
>> to fully automate it when we start with an CI merging system.
>>
> 
> I usually hack tasks like these with a special "editor" for git. I've attached
> one for Reviewed-By.
> 
> Usage:
> REVIEWER='I Myself <me at ego.example>' GIT_EDITOR=add-reviewed-by.py git commit
> --amend -e
> 

Thanks.

> 
> I'll use some time this week to write a better patch-pushing helper that'll
> incorporate this.
> (For the record, now we usually use
> https://github.com/mkosek/ipa-tools/blob/master/pushpatch.py)

That may be the best option for the short term. I would envision something like:

$ pushpatch.py freeipa-somebody-1-great.patch
...
Reviewed by:
0) Me
1) Petr Vobornik
2) Martin Kosek
3) Petr Viktorin
4) ...
99) Others:

Reviewed-By choice [0]: _

Martin