Skip to content

Commit

Permalink
Support for fixup and squash commits
Browse files Browse the repository at this point in the history
This code is related to #33.

- Added config code
- Added git parsing code

TODO: End-to-end unit and integration tests
  • Loading branch information
jroovers-cisco committed Dec 1, 2017
1 parent b054219 commit ac90a64
Show file tree
Hide file tree
Showing 14 changed files with 245 additions and 52 deletions.
66 changes: 40 additions & 26 deletions docs/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,51 +17,63 @@ The block below shows a sample ```.gitlint``` file. Details about rule config op

```ini
# All these sections are optional, edit this file as you like.
[general]
ignore=title-trailing-punctuation, T3
# [general]
# Ignore certain rules, you can reference them by their id or by their full name
# ignore=title-trailing-punctuation, T3

# verbosity should be a value between 1 and 3, the commandline -v flags take precedence over this
verbosity = 2
# verbosity = 2

# By default gitlint will ignore merge commits. Set to 'false' to disable.
ignore-merge-commits=true
# ignore-merge-commits=true

# By default gitlint will ignore fixup commits. Set to 'false' to disable.
# ignore-fixup-commits=true

# By default gitlint will ignore squash commits. Set to 'false' to disable.
# ignore-squash-commits=true

# Enable debug mode (prints more output). Disabled by default.
# debug=true

# Set the extra-path where gitlint will search for user defined rules
# See http://jorisroovers.github.io/gitlint/user_defined_rules for details
# extra-path=examples/

[title-max-length]
line-length=20
# [title-max-length]
# line-length=80

[title-must-not-contain-word]
# [title-must-not-contain-word]
# Comma-separated list of words that should not occur in the title. Matching is case
# insensitive. It's fine if the keyword occurs as part of a larger word (so "WIPING"
# will not cause a violation, but "WIP: my title" will.
words=wip,title
# words=wip

[title-match-regex]
# [title-match-regex]
# python like regex (https://docs.python.org/2/library/re.html) that the
# commit-msg title must be matched to.
# Note that the regex can contradict with other rules if not used correctly
# (e.g. title-must-not-contain-word).
regex=^US[0-9]*
# regex=^US[0-9]*

[B1]
# [B1]
# B1 = body-max-line-length
line-length=30
# line-length=120

[body-min-length]
min-length=5
# [body-min-length]
# min-length=5

[body-is-missing]
# [body-is-missing]
# Whether to ignore this rule on merge commits (which typically only have a title)
# default = True
ignore-merge-commits=false
# ignore-merge-commits=false

[body-changed-file-mention]
# [body-changed-file-mention]
# List of files that need to be explicitly mentioned in the body when they are changed
# This is useful for when developers often erroneously edit certain files or git submodules.
# By specifying this rule, developers can only change the file when they explicitly reference
# it in the commit message.
files=gitlint/rules.py,README.md
# files=gitlint/rules.py,README.md

# [author-valid-email]
# python like regex (https://docs.python.org/2/library/re.html) that the
Expand Down Expand Up @@ -109,11 +121,13 @@ The table below outlines configuration options that modify gitlint's overall beh
using commandline flags or in ```general``` section in a ```.gitlint``` configuration file.

Name | Default value | gitlint version | commandline flag | Description
---------------------|---------------|------------------|---------------------------------------|-------------------------------------
silent | false | >= 0.1 | ```--silent``` | Enable silent mode (no output). Use [exit](index.md#exit-codes) code to determine result.
verbosity | 3 | >= 0.1 | ```--verbosity=3``` | Amount of output gitlint will show when printing errors.
ignore-merge-commits | true | >= 0.7.0 | Not available | Whether or not to ignore merge commits.
ignore | [] (=empty) | >= 0.1 | ```--ignore=T1,body-min-length``` | Comma seperated list of rules to ignore (by name or id)
debug | false | >= 0.7.1 | ```--debug``` | Enable debugging output
target | (empty) | >= 0.8.0 | ```---target=/home/joe/myrepo/ ``` | Target git repository gitlint should be linting against.
extra-path | (empty) | >= 0.8.0 | ```---extra-path=/home/joe/rules/``` | Path where gitlint looks for [user-defined rules](user_defined_rules.md).
----------------------|---------------|------------------|---------------------------------------|-------------------------------------
silent | false | >= 0.1 | ```--silent``` | Enable silent mode (no output). Use [exit](index.md#exit-codes) code to determine result.
verbosity | 3 | >= 0.1 | ```--verbosity=3``` | Amount of output gitlint will show when printing errors.
ignore-merge-commits | true | >= 0.7.0 | Not available | Whether or not to ignore merge commits.
ignore-fixup-commits | true | >= 0.9.0 | Not available | Whether or not to ignore fixup commits.
ignore-squash-commits | true | >= 0.9.0 | Not available | Whether or not to ignore squash commits.
ignore | [] (=empty) | >= 0.1 | ```--ignore=T1,body-min-length``` | Comma seperated list of rules to ignore (by name or id)
debug | false | >= 0.7.1 | ```--debug``` | Enable debugging output
target | (empty) | >= 0.8.0 | ```---target=/home/joe/myrepo/ ``` | Target git repository gitlint should be linting against.
extra-path | (empty) | >= 0.8.0 | ```---extra-path=/home/joe/rules/``` | Path where gitlint looks for [user-defined rules](user_defined_rules.md).
2 changes: 1 addition & 1 deletion docs/rules.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ B4 | body-first-line-empty | >= 0.1 | First line of the body (
B5 | body-min-length | >= 0.4 | Body length must be at least 20 characters
B6 | body-is-missing | >= 0.4 | Body message must be specified
B7 | body-changed-file-mention | >= 0.4 | Body must contain references to certain files if those files are changed in the last commit
M1 | author-valid-email | >= 0.8.3 | Author email address must be a valid email address
M1 | author-valid-email | >= 0.9 | Author email address must be a valid email address



Expand Down
24 changes: 24 additions & 0 deletions gitlint/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,8 @@ def __init__(self):
self._rules = OrderedDict([(rule_cls.id, rule_cls()) for rule_cls in self.default_rule_classes])
self._verbosity = options.IntOption('verbosity', 3, "Verbosity")
self._ignore_merge_commits = options.BoolOption('ignore-merge-commits', True, "Ignore merge commits")
self._ignore_fixup_commits = options.BoolOption('ignore-fixup-commits', True, "Ignore fixup commits")
self._ignore_squash_commits = options.BoolOption('ignore-squash-commits', True, "Ignore squash commits")
self._debug = options.BoolOption('debug', False, "Enable debug mode")
self._extra_path = None
target_description = "Path of the target git repository (default=current working directory)"
Expand Down Expand Up @@ -103,6 +105,24 @@ def ignore_merge_commits(self):
def ignore_merge_commits(self, value):
return self._ignore_merge_commits.set(value)

@property
def ignore_fixup_commits(self):
return self._ignore_fixup_commits.value

@ignore_fixup_commits.setter
@handle_option_error
def ignore_fixup_commits(self, value):
return self._ignore_fixup_commits.set(value)

@property
def ignore_squash_commits(self):
return self._ignore_squash_commits.value

@ignore_squash_commits.setter
@handle_option_error
def ignore_squash_commits(self, value):
return self._ignore_squash_commits.set(value)

@property
def debug(self):
return self._debug.value
Expand Down Expand Up @@ -213,6 +233,8 @@ def __eq__(self, other):
self.target == other.target and \
self.extra_path == other.extra_path and \
self.ignore_merge_commits == other.ignore_merge_commits and \
self.ignore_fixup_commits == other.ignore_fixup_commits and \
self.ignore_squash_commits == other.ignore_squash_commits and \
self.debug == other.debug and \
self.ignore == other.ignore and \
self._config_path == other._config_path # noqa
Expand All @@ -224,6 +246,8 @@ def __str__(self):
return_str += u"extra-path: {0}\n".format(self.extra_path)
return_str += u"ignore: {0}\n".format(",".join(self.ignore))
return_str += u"ignore-merge-commits: {0}\n".format(self.ignore_merge_commits)
return_str += u"ignore-fixup-commits: {0}\n".format(self.ignore_fixup_commits)
return_str += u"ignore-squash-commits: {0}\n".format(self.ignore_squash_commits)
return_str += u"verbosity: {0}\n".format(self.verbosity)
return_str += u"debug: {0}\n".format(self.debug)
return_str += u"target: {0}\n".format(self.target)
Expand Down
10 changes: 10 additions & 0 deletions gitlint/files/gitlint
Original file line number Diff line number Diff line change
@@ -1,10 +1,20 @@
# All these sections are optional, edit this file as you like.
# [general]
# Ignore certain rules, you can reference them by their id or by their full name
# ignore=title-trailing-punctuation, T3

# verbosity should be a value between 1 and 3, the commandline -v flags take precedence over this
# verbosity = 2

# By default gitlint will ignore merge commits. Set to 'false' to disable.
# ignore-merge-commits=true

# By default gitlint will ignore fixup commits. Set to 'false' to disable.
# ignore-fixup-commits=true

# By default gitlint will ignore squash commits. Set to 'false' to disable.
# ignore-squash-commits=true

# Enable debug mode (prints more output). Disabled by default.
# debug=true

Expand Down
16 changes: 10 additions & 6 deletions gitlint/git.py
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ class GitCommit(object):
"""

def __init__(self, context, message, sha=None, date=None, author_name=None, author_email=None, parents=None,
is_merge_commit=False, changed_files=None):
is_merge_commit=None, is_fixup_commit=None, is_squash_commit=None, changed_files=None):
self.context = context
self.message = message
self.sha = sha
Expand All @@ -116,9 +116,13 @@ def __init__(self, context, message, sha=None, date=None, author_name=None, auth
self.author_email = author_email
# parent commit hashes
self.parents = parents or []
self.is_merge_commit = is_merge_commit
self.changed_files = changed_files or []

# If it's not explicitely specified, we consider a commit a merge commit if its title starts with "Merge"
self.is_merge_commit = message.title.startswith(u"Merge") if is_merge_commit is None else is_merge_commit
self.is_fixup_commit = message.title.startswith(u"fixup!") if is_fixup_commit is None else is_fixup_commit
self.is_squash_commit = message.title.startswith(u"squash!") if is_squash_commit is None else is_squash_commit

def __unicode__(self):
format_str = u"Author: %s <%s>\nDate: %s\n%s" # pragma: no cover
return format_str % (self.author_name, self.author_email, self.date, ustr(self.message)) # pragma: no cover
Expand All @@ -135,7 +139,8 @@ def __eq__(self, other):
self.sha == other.sha and self.author_name == other.author_name and \
self.author_email == other.author_email and \
self.date == other.date and self.parents == other.parents and \
self.is_merge_commit == other.is_merge_commit and self.changed_files == other.changed_files # noqa
self.is_merge_commit == other.is_merge_commit and self.is_fixup_commit == other.is_fixup_commit and \
self.is_squash_commit == other.is_squash_commit and self.changed_files == other.changed_files # noqa


class GitContext(object):
Expand All @@ -154,9 +159,7 @@ def from_commit_msg(commit_msg_str):
context = GitContext()
commit_msg_obj = GitCommitMessage.from_full_message(commit_msg_str)

# For now, we consider a commit a merge commit if its title starts with "Merge"
is_merge_commit = commit_msg_obj.title.startswith("Merge")
commit = GitCommit(context, commit_msg_obj, is_merge_commit=is_merge_commit)
commit = GitCommit(context, commit_msg_obj)

context.commits.append(commit)
return context
Expand Down Expand Up @@ -198,6 +201,7 @@ def from_local_repository(repository_path, refspec=None):

# Create Git commit object with the retrieved info
commit_msg_obj = GitCommitMessage.from_full_message(commit_msg)

commit = GitCommit(context, commit_msg_obj, sha=sha, author_name=name,
author_email=email, date=commit_date, changed_files=changed_files,
parents=commit_parents, is_merge_commit=commit_is_merge_commit)
Expand Down
10 changes: 7 additions & 3 deletions gitlint/lint.py
Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,13 @@ def lint(self, commit):
""" Lint the last commit in a given git context by applying all title, body and general rules. """
LOG.debug("Linting commit %s", commit.sha or "[SHA UNKNOWN]")
LOG.debug("Commit Object\n" + ustr(commit))
# Skip linting if this is merge commit and if the config is set to ignore those
if commit.is_merge_commit and self.config.ignore_merge_commits:
return []

# Skip linting if this is a special commit type that is configured to be ignored
ignore_commit_types = ["merge", "squash", "fixup"]
for commit_type in ignore_commit_types:
if getattr(commit, "is_{0}_commit".format(commit_type)) and \
getattr(self.config, "ignore_{0}_commits".format(commit_type)):
return []

violations = []
# determine violations by applying all rules
Expand Down
7 changes: 5 additions & 2 deletions gitlint/tests/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,10 +71,13 @@ def gitcontext(commit_msg_str, changed_files=None):
return gitcontext

@staticmethod
def gitcommit(commit_msg_str, changed_files=None):
def gitcommit(commit_msg_str, changed_files=None, **kwargs):
""" Utility method to easily create git commit given a commit msg string and an optional set of changed files"""
gitcontext = BaseTestCase.gitcontext(commit_msg_str, changed_files)
return gitcontext.commits[-1]
commit = gitcontext.commits[-1]
for attr, value in kwargs.iteritems():
setattr(commit, attr, value)
return commit

def assert_logged(self, lines):
""" Asserts that a certain list of messages has been logged """
Expand Down
2 changes: 2 additions & 0 deletions gitlint/tests/expected/debug_configuration_output1
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ config-path: {config_path}
extra-path: None
ignore: title-trailing-whitespace,B2
ignore-merge-commits: False
ignore-fixup-commits: True
ignore-squash-commits: True
verbosity: 1
debug: True
target: {target}
Expand Down
24 changes: 19 additions & 5 deletions gitlint/tests/test_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,9 @@ def test_set_general_option(self):

# Check that default general options are correct
self.assertTrue(config.ignore_merge_commits)
self.assertTrue(config.ignore_fixup_commits)
self.assertTrue(config.ignore_squash_commits)

self.assertFalse(config.debug)
self.assertEqual(config.verbosity, 3)
active_rule_classes = tuple(type(rule) for rule in config.rules)
Expand All @@ -82,6 +85,14 @@ def test_set_general_option(self):
config.set_general_option("ignore-merge-commits", "false")
self.assertFalse(config.ignore_merge_commits)

# ignore_fixup_commit
config.set_general_option("ignore-fixup-commits", "false")
self.assertFalse(config.ignore_fixup_commits)

# ignore_squash_commit
config.set_general_option("ignore-squash-commits", "false")
self.assertFalse(config.ignore_squash_commits)

# debug
config.set_general_option("debug", "true")
self.assertTrue(config.debug)
Expand Down Expand Up @@ -148,12 +159,15 @@ def test_set_general_option_negative(self):
with self.assertRaisesRegex(LintConfigError, "Option 'verbosity' must be set between 0 and 3"):
config.verbosity = value

# invalid ignore_merge_commits
# invalid ignore_xxx_commits
ignore_attributes = ['ignore_merge_commits', 'ignore_fixup_commits', 'ignore_squash_commits']
incorrect_values = [-1, 4, u"föo"]
for value in incorrect_values:
with self.assertRaisesRegex(LintConfigError,
"Option 'ignore-merge-commits' must be either 'true' or 'false'"):
config.ignore_merge_commits = value
for attribute in ignore_attributes:
for value in incorrect_values:
option_name = attribute.replace("_", "-")
with self.assertRaisesRegex(LintConfigError,
"Option '{0}' must be either 'true' or 'false'".format(option_name)):
setattr(config, attribute, value)

# invalid ignore -> not here because ignore is a ListOption which converts everything to a string before
# splitting which means it it will accept just about everything
Expand Down

0 comments on commit ac90a64

Please sign in to comment.