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

[Jakub Hrozek]

On Mon, Feb 10, 2014 at 01:59:27PM +0100, Martin Kosek wrote:
> 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

For SSSD I simply added a ~/.vimrc snippet based on:
https://wiki.samba.org/index.php/CodeReview

It currently includes:
function! CommitMessages()
    nmap R iReviewed-by: Jakub Hrozek <jhrozek at redhat.com><CR><ESC>
    iab #R Reviewed-by:
    iab JH Jakub<SPACE>Hrozek<SPACE><jhrozek at redhat.com>
    (and similar entries for other developers)
endf
autocmd BufWinEnter COMMIT_EDITMSG,*.diff,*.patch,*.patches.txt call CommitMessages()

Of course, this wouldn't work if you're using an inferior editor :-]