pre-upload: enforce some style on commit messages
Some commit message style checks can be automated, so start doing that
rather than forcing our reviewers to do it.
BUG=None
TEST=`./pre-upload_unittest.py` passes
Change-Id: Ic4e4a56b7cfd3576ce390fca36adfdf57e2069d4
Reviewed-on: https://chromium-review.googlesource.com/226682
Tested-by: Mike Frysinger <vapier@chromium.org>
Reviewed-by: Don Garrett <dgarrett@chromium.org>
Commit-Queue: Mike Frysinger <vapier@chromium.org>
diff --git a/pre-upload.py b/pre-upload.py
index b44cbc9..fa964fb 100755
--- a/pre-upload.py
+++ b/pre-upload.py
@@ -714,6 +714,30 @@
return HookFailure('Change-Id must be in last paragraph of description.')
+def _check_commit_message_style(_project, commit):
+ """Verify that the commit message matches our style.
+
+ We do not check for BUG=/TEST=/etc... lines here as that is handled by other
+ commit hooks.
+ """
+ desc = _get_commit_desc(commit)
+
+ # The first line should be by itself.
+ lines = desc.splitlines()
+ if len(lines) > 1 and lines[1]:
+ return HookFailure('The second line of the commit message must be blank.')
+
+ # The first line should be one sentence.
+ if '. ' in lines[0]:
+ return HookFailure('The first line cannot be more than one sentence.')
+
+ # The first line cannot be too long.
+ MAX_FIRST_LINE_LEN = 100
+ if len(lines[0]) > MAX_FIRST_LINE_LEN:
+ return HookFailure('The first line must be less than %i chars.' %
+ MAX_FIRST_LINE_LEN)
+
+
def _check_license(_project, commit):
"""Verifies the license/copyright header.
@@ -1017,6 +1041,7 @@
_check_change_has_valid_cq_depend,
_check_change_has_test_field,
_check_change_has_proper_changeid,
+ _check_commit_message_style,
]
diff --git a/pre-upload_unittest.py b/pre-upload_unittest.py
index eddbb7e..815308f 100755
--- a/pre-upload_unittest.py
+++ b/pre-upload_unittest.py
@@ -642,5 +642,31 @@
self.assertMessageRejected('\nChange-Id: I1234\nbar\n')
+class CheckCommitMessageStyle(CommitMessageTestCase):
+ """Tests for _check_commit_message_style."""
+
+ @staticmethod
+ def CheckMessage(project, commit):
+ return pre_upload._check_commit_message_style(project, commit)
+
+ def testNormal(self):
+ """Accept valid commit messages."""
+ self.assertMessageAccepted('one sentence.\n')
+ self.assertMessageAccepted('some.module: do it!\n')
+ self.assertMessageAccepted('one line\n\nmore stuff here.')
+
+ def testNoBlankSecondLine(self):
+ """Reject messages that have stuff on the second line."""
+ self.assertMessageRejected('one sentence.\nbad fish!\n')
+
+ def testFirstLineMultipleSentences(self):
+ """Reject messages that have more than one sentence in the summary."""
+ self.assertMessageRejected('one sentence. two sentence!\n')
+
+ def testFirstLineTooLone(self):
+ """Reject first lines that are too long."""
+ self.assertMessageRejected('o' * 200)
+
+
if __name__ == '__main__':
cros_test_lib.main()
