From 881e3729c4627dcc71e63343538e4ec470a7c23f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Erik=20Nordstr=C3=B6m?= Date: Fri, 5 Apr 2019 14:41:23 +0200 Subject: [PATCH] Add a Git commit hook to validate commit messages Our coding guidelines recommend following "The seven rules of a great Git commit message" by Chris Beams: https://chris.beams.io/posts/git-commit/ This change adds a Git commit hook that validates all Git commit messages according to these rules (at least to the extent possible). Currently, the hook simply prints a warning and a list of violations in case of a non-conforming Git commit message. The commit is otherwise accepted. This could be changed to entirely fail the commit, or, via another hook, fail to push any code that is non-conformant. The hook will be installed on a CMake run or when the hook source changes. --- .travis.yml | 11 ++ CMakeLists.txt | 4 + scripts/githooks/commit_msg.py | 197 +++++++++++++++++++++++++++ scripts/githooks/commit_msg_tests.py | 71 ++++++++++ 4 files changed, 283 insertions(+) create mode 100755 scripts/githooks/commit_msg.py create mode 100755 scripts/githooks/commit_msg_tests.py diff --git a/.travis.yml b/.travis.yml index 83944461e..d7510e762 100644 --- a/.travis.yml +++ b/.travis.yml @@ -68,6 +68,17 @@ jobs: - git diff --exit-code - docker exec pgbuild /bin/bash -c "/tsdb_build/scripts/export_prefix_check.sh" + - if: (type = cron) OR NOT (branch = master) + stage: test + name: "Git hook tests" + before_install: + install: + after_failure: + after_script: + script: + - python2 ./scripts/githooks/commit_msg_tests.py + - python3 ./scripts/githooks/commit_msg_tests.py + - if: (type = pull_request) OR (type = cron) OR NOT (branch = master) stage: test name: "Regression 9.6" diff --git a/CMakeLists.txt b/CMakeLists.txt index 564644236..69fba1e1d 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -326,3 +326,7 @@ endif() add_custom_target(licensecheck COMMAND ${PROJECT_SOURCE_DIR}/scripts/check_license_all.sh ) + +if (IS_DIRECTORY ${PROJECT_SOURCE_DIR}/.git) + configure_file(${PROJECT_SOURCE_DIR}/scripts/githooks/commit_msg.py ${PROJECT_SOURCE_DIR}/.git/hooks/commit-msg COPYONLY) +endif() diff --git a/scripts/githooks/commit_msg.py b/scripts/githooks/commit_msg.py new file mode 100755 index 000000000..7bf2fd98b --- /dev/null +++ b/scripts/githooks/commit_msg.py @@ -0,0 +1,197 @@ +#!/usr/bin/env python + +# Check a Git commit message according to the seven rules of a good commit message: +# https://chris.beams.io/posts/git-commit/ +import sys + +class GitCommitMessage: + 'Represents a parsed Git commit message' + + rules = [ 'Separate subject from body with a blank line', + 'Limit the subject line to 50 characters', + 'Capitalize the subject line', + 'Do not end the subject line with a period', + 'Use the imperative mood in the subject line', + 'Wrap the body at 72 characters', + 'Use the body to explain what and why vs. how' ] + + valid_rules = [ False, False, False, False, False, False, False ] + + def __init__(self, filename = None): + lines = [] + + if filename != None: + with open(filename, 'r') as f: + for line in f: + if not line.startswith('#'): + lines.append(line) + + self.parse_lines(lines) + + def parse_lines(self, lines): + self.body_lines = [] + self.subject = [] + + if not lines or len(lines) == 0: + return self + + self.subject = lines[0] + self.subject_words = self.subject.split() + self.has_subject_body_separator = False + + if len(lines) > 1: + self.has_subject_body_separator = (len(lines[1].strip()) == 0) + + if self.has_subject_body_separator: + self.body_lines = lines[2:] + else: + self.body_lines = lines[1:] + + return self + + def check_subject_body_separtor(self): + 'Rule 1: Separate subject from body with a blank line' + + if len(self.body_lines) > 0: + return self.has_subject_body_separator + return True + + def check_subject_limit(self): + 'Rule 2: Limit the subject line to 50 characters' + return len(self.subject) <= 50 + + def check_subject_capitalized(self): + 'Rule 3: Capitalize the subject line' + return len(self.subject) > 0 and self.subject[0].isupper() + + def check_subject_no_period(self): + 'Rule 4: Do not end the subject line with a period' + return not self.subject.endswith('.') + + common_first_words = [ 'Add', + 'Adjust', + 'Support', + 'Change', + 'Remove', + 'Fix', + 'Print', + 'Track', + 'Refactor', + 'Combine', + 'Release', + 'Set', + 'Stop', + 'Make', + 'Mark', + 'Enable', + 'Check', + 'Exclude', + 'Format', + 'Correct'] + + def check_subject_imperative(self): + 'Rule 5: Use the imperative mood in the subject line.' + 'We can only check for common mistakes here, like using' + 'the -ing form of a verb or non-imperative version of ' + 'common verbs' + + firstword = self.subject_words[0] + + if firstword.endswith('ing'): + return False + + for word in self.common_first_words: + if firstword.startswith(word) and firstword != word: + return False + + return True + + def check_body_limit(self): + 'Rule 6: Wrap the body at 72 characters' + + if len(self.body_lines) == 0: + return True + + for line in self.body_lines: + if len(line) > 72: + return False + + return True + + def check_body_uses_why(self): + 'Rule 7: Use the body to explain what and why vs. how' + # Not enforcable + return True + + rule_funcs = [ + check_subject_body_separtor, + check_subject_limit, + check_subject_capitalized, + check_subject_no_period, + check_subject_imperative, + check_body_limit, + check_body_uses_why ] + + def check_the_seven_rules(self): + 'validates the commit message against the seven rules' + + num_violations = 0 + + for i in range(len(self.rule_funcs)): + func = self.rule_funcs[i] + res = func(self) + self.valid_rules[i] = res + + if not res: + num_violations += 1 + + if num_violations > 0: + print + print("**** WARNING ****") + print + print("The commit message does not seem to comply with the project's guidelines.") + print("Please try to follow the \"Seven rules of a great commit message\":") + print("https://chris.beams.io/posts/git-commit/") + print + print("The following rules are violated:\n") + + for i in range(len(self.rule_funcs)): + if not self.valid_rules[i]: + print("\t* Rule %d: \"%s\"" % (i+1, self.rules[i])) + + # Extra sanity checks beyond the seven rules + if len(self.body_lines) == 0: + print + print("NOTE: the commit message has no body.") + print("It is recommended to add a body with a description of your") + print("changes, even if they are small. Explain what and why instead of how:") + print("https://chris.beams.io/posts/git-commit/#why-not-how") + + if len(self.subject_words) < 3: + print + print("Warning: the subject line has less than three words.") + print("Consider using a more explanatory subject line.") + + if num_violations > 0: + print + print("Run 'git commit --amend' to change the commit message") + + print + + return num_violations + +def main(): + if len(sys.argv) != 2: + print("Unexpected number of arguments") + exit(1) + + msg = GitCommitMessage(sys.argv[1]) + return msg.check_the_seven_rules() + +if __name__ == "__main__": + main() + # Always exit with success. We could also fail the commit if with + # a non-zero exit code, but that might be a bit excessive and we'd + # have to save the failed commit message to a file so that it can + # be recovered. + exit(0) diff --git a/scripts/githooks/commit_msg_tests.py b/scripts/githooks/commit_msg_tests.py new file mode 100755 index 000000000..6373c89b2 --- /dev/null +++ b/scripts/githooks/commit_msg_tests.py @@ -0,0 +1,71 @@ +#!/usr/bin/env python + +import unittest +from commit_msg import GitCommitMessage + +class TestCommitMsg(unittest.TestCase): + + def testNoInput(self): + m = GitCommitMessage().parse_lines([]) + self.assertEqual(len(m.body_lines), 0) + m = GitCommitMessage().parse_lines(None) + self.assertEqual(len(m.body_lines), 0) + + def testParsing(self): + m = GitCommitMessage().parse_lines(['This is a subject line', ' ', 'body line 1', 'body line 2']) + self.assertEqual(m.subject, 'This is a subject line') + self.assertTrue(m.has_subject_body_separator) + self.assertEqual(m.body_lines[0], 'body line 1') + self.assertEqual(m.body_lines[1], 'body line 2') + + def testNonImperative(self): + m = GitCommitMessage().parse_lines(['Adds new file']) + self.assertFalse(m.check_subject_imperative()) + + m.parse_lines(['Adding new file']) + self.assertFalse(m.check_subject_imperative()) + + # Default to accept + m.parse_lines(['Foo bar']) + self.assertTrue(m.check_subject_imperative()) + + def testSubjectBodySeparator(self): + m = GitCommitMessage().parse_lines(['This is a subject line']) + self.assertTrue(m.check_subject_body_separtor()) + m = GitCommitMessage().parse_lines(['This is a subject line', 'body']) + self.assertFalse(m.check_subject_body_separtor()) + m = GitCommitMessage().parse_lines(['This is a subject line', '', 'body']) + self.assertTrue(m.check_subject_body_separtor()) + m = GitCommitMessage().parse_lines(['This is a subject line', ' ', 'body']) + self.assertTrue(m.check_subject_body_separtor()) + + def testSubjectLimit(self): + m = GitCommitMessage().parse_lines(['This subject line is exactly 48 characters long']) + self.assertTrue(m.check_subject_limit()) + m = GitCommitMessage().parse_lines(['This is a very long subject line that will obviously exceed the limit']) + self.assertFalse(m.check_subject_limit()) + + def testSubjectCapitalized(self): + m = GitCommitMessage().parse_lines(['This subject line is capitalized']) + self.assertTrue(m.check_subject_capitalized()) + m = GitCommitMessage().parse_lines(['this subject line is not capitalized']) + self.assertFalse(m.check_subject_capitalized()) + + def testSubjectNoPeriod(self): + m = GitCommitMessage().parse_lines(['This subject line ends with a period.']) + self.assertFalse(m.check_subject_no_period()) + m = GitCommitMessage().parse_lines(['This subject line does not end with a period']) + self.assertTrue(m.check_subject_no_period()) + + def testBodyLimit(self): + m = GitCommitMessage().parse_lines(['This is a subject line', ' ', 'A short body line']) + self.assertTrue(m.check_body_limit()) + m = GitCommitMessage().parse_lines(['This is a subject line', ' ', 'A very long body line which certainly exceeds the 72 char recommended limit']) + self.assertFalse(m.check_body_limit()) + + def testCheckAllRules(self): + m = GitCommitMessage().parse_lines(['This is a subject line', '', 'A short body line']) + self.assertEqual(0, m.check_the_seven_rules()) + +if __name__ == "__main__": + unittest.main()