-
Notifications
You must be signed in to change notification settings - Fork 4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Turn off commit-msg hook enforcing certain commit format, and add branch name prepender #4
Conversation
…nch name prepender Both are disabled by default. Issue: https://khanacademy.atlassian.net/browse/FEI-3686 Test plan: ka-clone something, write a one-line commit message, and it should succeed! Copy over the branch-name hook from the ka-clone repo into the .git/hooks directory, do `git commit --allow-empty -m "hello"`, and the branch name should be prepended (unless you're on master or main)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In general, I love this, it's a great step forward. A couple of small nits, but nothing that's a show-stopper, so fix here or in a follow-up, or never if that's what makes the most sense.
First problem I notice is that if I git commit
without the -m
, then quit the editor in the commit message without saving, it actually commits the template as the message.
commit 2bcaf25afb08adb9f78089efb4f4b889a20fcc81 (HEAD -> test-ka-clone)
Author: David Braley <d***@redacted.com>
Date: Tue Aug 3 15:46:50 2021 -0400
[test-ka-clone] <one-line summary>
<full summary>
Issue: <url or "none">
Test plan:
<see below>
Other nits in line.
None of this is show-stopper though, so move forward as you see fit.
templates/commit-msg.lint
Outdated
@@ -1,4 +1,6 @@ | |||
#!/bin/sh | |||
# NOTE: This linter is disabled by default. If you want to enable it, | |||
# uncomment the last line of the `commit-msg` hook. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not entirely clear which line this is referring to.
bin/ka-clone
Outdated
parser.add_argument('-p', '--protect-master', | ||
action='store_true', | ||
help='install hooks to protect the master branch') | ||
parser.add_argument('--repair', | ||
parser.add_argument('--branch-name', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Once I've enabled this option, I don't seem to be able to disable it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So excited! We can use github markdown in commit messages after this! 😱
@dbraley hmm yeah that's weird; I'm not sure off the top of my head how to fix that 🤔 looks like git is comparing the message against the template, which I might be able to do as well, if it lived in a standard place. I'll make a ticket to see if I can make that better |
Summary:
Both are disabled by default.
Issue: https://khanacademy.atlassian.net/browse/FEI-3686
Test plan:
./bin/ka-clone --repair
./bin/ka-clone --repair --branch-name-hook
git checkout -b some-branch
git commit --allow-empty -m hello
git log
should show that the branch name has been prepended!