[SSSD] [PATCH] Add a client-side hook to prevent pushes without Reviewed-By

Jakub Hrozek jhrozek at redhat.com
Sun Oct 4 19:56:01 UTC 2015


On Fri, Oct 02, 2015 at 01:51:18PM +0200, Lukas Slebodnik wrote:
> On (02/10/15 13:02), Lukas Slebodnik wrote:
> >On (30/09/15 09:22), Jakub Hrozek wrote:
> >>On Tue, Sep 29, 2015 at 08:28:30AM +0200, Lukas Slebodnik wrote:
> >>> On (28/09/15 14:19), Jakub Hrozek wrote:
> >>> >Hi,
> >>> >
> >>> >to activate this hook, copy it from contrib to .git/hooks and make sure
> >>> >the executable flag is on. Attempting to push a commit without
> >>> >Reviewed-By will then trigger an error.
> >>> >
> >>> Good idea.
> >>> 
> >>> >If we want to be truly strict about not pushing commits without a RB
> >>> >tag, then we need a server-side hook.
> >>> As you wish.
> >>> 
> >>> 
> >>> >From 391666fda49fef9ac003d192e7ae8a3c2b00e113 Mon Sep 17 00:00:00 2001
> >>> >From: Jakub Hrozek <jhrozek at redhat.com>
> >>> >Date: Mon, 28 Sep 2015 13:46:39 +0200
> >>> >Subject: [PATCH] contrib: Add a pre-push hook to warn about commits without
> >>> > Reviewed-By
> >>> >
> >>> >---
> >>> > contrib/git/pre-push | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> >>> > 1 file changed, 62 insertions(+)
> >>> > create mode 100755 contrib/git/pre-push
> >>> >
> >>> >diff --git a/contrib/git/pre-push b/contrib/git/pre-push
> >>> Initially, I thought it does not work because I tested with
> >>> creating a new branch
> >>>   git push origin master:new_branch
> >>> However it works after trying to push another patch to this branch.
> >>> 
> >>> There are few pep8 warning
> >>> sh$ pep8 contrib/git/pre-push
> >>> contrib/git/pre-push:10:1: E302 expected 2 blank lines, found 1
> >>> contrib/git/pre-push:11:13: E201 whitespace after '['
> >>> contrib/git/pre-push:11:70: E202 whitespace before ']'
> >>> contrib/git/pre-push:14:13: E201 whitespace after '['
> >>> contrib/git/pre-push:14:74: E202 whitespace before ']'
> >>> contrib/git/pre-push:16:1: E302 expected 2 blank lines, found 1
> >>> contrib/git/pre-push:17:13: E201 whitespace after '['
> >>> contrib/git/pre-push:17:54: E202 whitespace before ']'
> >>> contrib/git/pre-push:22:1: E302 expected 2 blank lines, found 1
> >>> contrib/git/pre-push:31:1: E302 expected 2 blank lines, found 1
> >>> contrib/git/pre-push:39:1: E302 expected 2 blank lines, found 1
> >>
> >>pep8 should be happy about the attached patch.
> >>
> >>> 
> >>> It would be also good to add small howto from mail.
> >>> "copy it from contrib to .git/hooks and make sure
> >>> the executable flag is on" to the commit message or better to
> >>> README into directory contrib/git/
> >>
> >>OK, Added.
> >>
> >>> 
> >>> and you can also mention hint about .git-commit-template
> >>
> >>I tried to add something, hope it makes sense.
> >
> >>From 66df6b096bcb3044431b1d65cb426a1f1fb1f37c Mon Sep 17 00:00:00 2001
> >>From: Jakub Hrozek <jhrozek at redhat.com>
> >>Date: Mon, 28 Sep 2015 13:46:39 +0200
> >>Subject: [PATCH] contrib: Add a pre-push hook to warn about commits without
> >> Reviewed-By
> >>
> >>---
> >> contrib/git/pre-push | 73 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> >> 1 file changed, 73 insertions(+)
> >> create mode 100755 contrib/git/pre-push
> >>
> >hmm,
> >it worked for me when I was pushing from one local git to
> >another local git.
> >
> >However, I tried to push this patch to upstream
> >and I was allowed to push it without review-by tag.
> >
> >master:
> >* 43147a9abaa1254235b853e643514cf1c7b150d7
> >
> >But we still need to find out why it didn't work for me
> >in the second case.
> >
> I found out a reason. There is an explicit check for
> # Don't warn when pushing to personal repositories, only origin
> remote = sys.argv[1]
> if remote != 'origin':
>     sys.exit(0)
> 
> 
> I had origin as https and for pushing I used different name
> with ssh protocol. I will change it.
> 
> But there is a small update for python3 :-)
> I'm sorry I didn't plan to push it so early.
> 
> LS

Works for me,

ACK


More information about the sssd-devel mailing list