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

Lukas Slebodnik lslebodn at redhat.com
Fri Oct 2 11:51:18 UTC 2015


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
-------------- next part --------------
>From fb3108ebd6b350fcfe06392921c3d1e53f2a7e8e Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik <lslebodn at redhat.com>
Date: Fri, 2 Oct 2015 13:25:22 +0200
Subject: [PATCH] CONTRIB: pre-push hook could work with python3

p.communicate() return bytes on python3
---
 contrib/git/pre-push | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/contrib/git/pre-push b/contrib/git/pre-push
index d05202dfdd2f59a885b843e25e89878d985909e4..a3bb85ed257de7320fe65d9375edf59a134db3d8 100755
--- a/contrib/git/pre-push
+++ b/contrib/git/pre-push
@@ -16,14 +16,14 @@ def get_all_commits(ref_from, ref_to):
     args = ['git', 'rev-list', '{:s}..{:s}'.format(ref_from, ref_to)]
     p = subprocess.Popen(args, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
     out, err = p.communicate()
-    return [commit.strip() for commit in out.split('\n') if commit != '']
+    return [commit.strip() for commit in out.decode('UTF-8').split('\n') if commit != '']
 
 
 def commit_message(commit_hash):
     args = ['git', 'cat-file', 'commit', commit_hash]
     p = subprocess.Popen(args, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
     out, err = p.communicate()
-    return out
+    return out.decode('UTF-8')
 
 
 def commit_has_rb(commit):
-- 
2.5.0



More information about the sssd-devel mailing list