Skip to content

Conversation

@sebastian-philipp
Copy link

@sebastian-philipp sebastian-philipp commented Oct 13, 2021

Fixes #22

Copy link
Contributor

@epuertat epuertat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The pre-commit hook for adding the "Resolves: rhbz#..." is completely unnecessary. It can easily be performed with the git interpret-trailers command.

Comment on lines +29 to +44
def which(executable):
locations = (
'/usr/local/bin',
'/bin',
'/usr/bin',
'/usr/local/sbin',
'/usr/sbin',
'/sbin',
)

for location in locations:
executable_path = os.path.join(location, executable)
if os.path.exists(executable_path):
return executable_path

GIT = which('git')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shutil.which()?

Comment on lines +67 to +96
def prepend_commit_msg(branch):
"""Prepend the commit message with `text`"""
msgfile = sys.argv[1]
with open(msgfile) as f:
contents = f.read()

if not branch:
return contents

try:
prefix, ticket_id = branch.split('-')
ticket_id = int(ticket_id)
except ValueError:
# We used to raise here, but if cherry-picking to a different branch
# that doesn't comply it would break preventing the cherry-pick. So we
# print out the warning, but end up returning the contents.
print('skipping commit msg change: Branch name "%s" does not follow required format: {tracker}-{ID}' % branch) # NOQA
return contents

if prefix == 'wip':
# Skip "wip" branches, ie "work in progress"
return contents

resolves_line = "\nResolves: %s#%s" % (prefix.lower(), ticket_id)

with open(msgfile, 'a') as f:
# Don't append if it's already there
if resolves_line not in contents:
f.write(resolves_line)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As we're invoking the git command, this could be easily done with git interpret-trailers command:

subprocess.run(['git', 'interpret-trailers', f'--trailer Resolves: rhbz#{ticket_id}', '--in-place', sys.argv[1]], shell=True, check=True)

That said, a pre-commit hook is not needed for this, and IMHO it should rather be done with interepreter trailers:

-    git('-c', 'core.editor=/bin/true', 'cherry-pick', '-x', '%s~..%s' % (first, last))
+    git('-c', 'core.editor=/bin/true', 'cherry-pick', '-x', '--no-commit', '%s~..%s' % (first, last))
+    git('commit', '--trailer "Resolves: rhbz#{$bz}"', '--no-edit')
    # Merge this rhbz branch back into our starting branch.
    git('checkout', starting_branch)
    git('merge', '--ff-only', 'rhbz-' + bz)
    git('branch', '-d', 'rhbz-' + bz)

@sebastian-philipp
Copy link
Author

also the hook fails:

Traceback (most recent call last):
  File "/home/sebastian/Repos/ceph/.git/hooks/prepare-commit-msg", line 49, in branch_name
    name = subprocess.check_output(
  File "/usr/lib64/python3.9/subprocess.py", line 424, in check_output
    return run(*popenargs, stdout=PIPE, timeout=timeout, check=True,
  File "/usr/lib64/python3.9/subprocess.py", line 528, in run
    raise CalledProcessError(retcode, process.args,
subprocess.CalledProcessError: Command '['/bin/git', 'symbolic-ref', 'HEAD']' returned non-zero exit status 128.

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/sebastian/Repos/ceph/.git/hooks/prepare-commit-msg", line 103, in <module>
    main()
  File "/home/sebastian/Repos/ceph/.git/hooks/prepare-commit-msg", line 99, in main
    branch = branch_name().strip().strip('\n')
  File "/home/sebastian/Repos/ceph/.git/hooks/prepare-commit-msg", line 53, in branch_name
    if 'fatal: ref HEAD is not a symbolic ref' in err.output:
TypeError: a bytes-like object is required, not 'str'
error: 'prepare-commit-msg' Hook fehlgeschlagen.

@ktdreyer
Copy link
Contributor

Are we ok with using #24 instead?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Python 3 Syntax Error (print)

3 participants