Arggghhh. Missing 'Signed-off-by:' and 'Reviewed-by:' in revert I pushed...

Martin Schwenke martin at meltin.net
Thu Oct 22 04:24:29 UTC 2015


[Sorry to those getting this twice.  The old message I replied to
 didn't have the modern lists.samba.org address for samba-technical.]

On Tue, 15 Jul 2014 14:50:40 -0700, Jeremy Allison <jra at samba.org>
wrote:

> On Wed, Jul 16, 2014 at 07:37:05AM +1000, Martin Schwenke wrote:
> > On Tue, 15 Jul 2014 09:26:35 -0700, Jeremy Allison <jra at samba.org>
> > wrote:
> >   
> > > On Tue, Jul 15, 2014 at 10:01:50AM +0200, Kai Blin wrote:  
>  [...]  
> > > 
> > > That's a really good idea - thanks !  
> > 
> > If you do this, can you please update the autobuild documentation to
> > show how this is done?  
> 
> I think it requires git 1.8.2 or above.
> 
> Here's a sample pre-push hook script:
> 
> https://github.com/git/git/blob/87c86dd14abe8db7d00b0df5661ef8cf147a72a3/templates/hooks--pre-push.sample
> 
> I'm planning to work from that.

I saw someone else revert something missing a "Reviewed-by:" recently,
so I decided to have a go at this.  I'll paste it here first to see if
anyone has any comments:

#!/bin/sh

check_for_reviewed_by ()
{
    fail=false

    for i in $(git rev-list @{upstream}..)  ; do
	if ! git show --pretty='format:%B' "$i" | grep -q '^Reviewed-by: ' ; then
	    echo "Commit ${i} is missing Reviewed-by:"
	    fail=true
	fi
    done

    if $fail ; then
	exit 1
    fi
}

remote="$1"
url="$2"

case "$remote" in
    autobuild)
	check_for_reviewed_by
esac

#

I've smoke tested this after installing it into my Samba
repository at .git/hooks/pre-push (and confirming that this is
executable).

I decided not to use any of the information provided to the hook on
stdin, since I don't want to check through commits that are already
upstream.  Instead, this depends on the upstream branch being set to
origin/master for the branch being pushed, so it checks all commits on
top of upstream.

Comments?  :-)

peace & happiness,
martin



More information about the samba-technical mailing list