Skip to content
This repository has been archived by the owner on Jan 31, 2018. It is now read-only.

[bug 1108755] Add a git commit message linter #422

Merged
merged 1 commit into from Dec 18, 2014

Conversation

lgp171188
Copy link
Contributor

As mentioned in the comments of bug 1108755 as requirements,
the commit message linter does the following things:

  1. Enforces max. 79 characters per line
  2. Verifies the summary line has bug numbers in the right format
  3. Does a bugzilla lookup for the bug and prints out the 'bug id',
    'summary' and 'assigned to' fields.

def check_bug_number_in_right_format(contents):
summary = contents[0]
if re.compile(r'\bbug\b', flags=re.IGNORECASE).search(summary) and \
re.search(r'\b#{0,1}\d+\b', summary):
Copy link
Member

Choose a reason for hiding this comment

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

Nit: This line is indented 3 spaces and the preceeding line has a \ in it. The fjord code tends to do things like:

if (re.compile(...) and
        re.search(r' ...)):

in this situation.

@willkg
Copy link
Member

willkg commented Dec 18, 2014

The nits are very minor and not a big deal.

However, when I run this in the vagrant vm, I get this:

(fjordvagrant)vagrant@vagrant-ubuntu-trusty-64:~/fjord$ bin/lint_commit_msg.py COMMIT_MSG 
Traceback (most recent call last):
  File "bin/lint_commit_msg.py", line 7, in <module>
    from requests.exceptions import (
ImportError: cannot import name ConnectTimeout

I think we need to change the code so it works with requests 1.1 which is what's installed in the vagrant vm.


if __name__ == "__main__":
commit_msg_file = sys.argv[1]
lint_commit_msg(commit_msg_file)
Copy link
Member

Choose a reason for hiding this comment

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

You should have lint_commit_msg return an int and then have the sys.exit() call here. That way the script doesn't short-circuit and it's easier to test.

As mentioned in the comments of bug 1108755 as requirements,
the commit message linter does the following things:

1. Enforces max. 79 characters per line
2. Verifies the summary line has bug numbers in the right format
3. Does a bugzilla lookup for the bug and prints out the 'bug id',
   'summary' and 'assigned to' fields.
@willkg
Copy link
Member

willkg commented Dec 18, 2014

This looks great! Thank you!

willkg added a commit that referenced this pull request Dec 18, 2014
[bug 1108755] Add a git commit message linter
@willkg willkg merged commit f0ec99d into mozilla:master Dec 18, 2014
@willkg
Copy link
Member

willkg commented Dec 18, 2014

a36a231 [bug 1108755] Add a git commit message linter

@lgp171188 lgp171188 deleted the 1108755-commit-lint branch December 19, 2014 13:06
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
2 participants