summaryrefslogtreecommitdiff
path: root/danger
diff options
context:
space:
mode:
authorGitLab Bot <gitlab-bot@gitlab.com>2020-01-16 18:08:46 +0000
committerGitLab Bot <gitlab-bot@gitlab.com>2020-01-16 18:08:46 +0000
commitaa0f0e992153e84e1cdec8a1c7310d5eb93a9f8f (patch)
tree4a662bc77fb43e1d1deec78cc7a95d911c0da1c5 /danger
parentd47f9d2304dbc3a23bba7fe7a5cd07218eeb41cd (diff)
downloadgitlab-ce-aa0f0e992153e84e1cdec8a1c7310d5eb93a9f8f.tar.gz
Add latest changes from gitlab-org/gitlab@master
Diffstat (limited to 'danger')
-rw-r--r--danger/changelog/Dangerfile8
-rw-r--r--danger/commit_messages/Dangerfile325
2 files changed, 102 insertions, 231 deletions
diff --git a/danger/changelog/Dangerfile b/danger/changelog/Dangerfile
index 8c010accd56..1c4647121fb 100644
--- a/danger/changelog/Dangerfile
+++ b/danger/changelog/Dangerfile
@@ -38,9 +38,13 @@ rescue StandardError => e
warn "There was a problem trying to check the Changelog. Exception: #{e.name} - #{e.message}"
end
+def sanitized_mr_title
+ helper.sanitize_mr_title(gitlab.mr_json["title"])
+end
+
if git.modified_files.include?("CHANGELOG.md")
fail "**CHANGELOG.md was edited.** Please remove the additions and create a CHANGELOG entry.\n\n" +
- format(CREATE_CHANGELOG_MESSAGE, mr_iid: gitlab.mr_json["iid"], mr_title: changelog.sanitized_mr_title, labels: changelog.presented_no_changelog_labels)
+ format(CREATE_CHANGELOG_MESSAGE, mr_iid: gitlab.mr_json["iid"], mr_title: sanitized_mr_title, labels: changelog.presented_no_changelog_labels)
end
changelog_found = changelog.found
@@ -50,6 +54,6 @@ if changelog.needed?
check_changelog(changelog_found)
else
message "**[CHANGELOG missing](https://docs.gitlab.com/ce/development/changelog.html)**: If this merge request [doesn't need a CHANGELOG entry](https://docs.gitlab.com/ee/development/changelog.html#what-warrants-a-changelog-entry), feel free to ignore this message.\n\n" +
- format(CREATE_CHANGELOG_MESSAGE, mr_iid: gitlab.mr_json["iid"], mr_title: changelog.sanitized_mr_title, labels: changelog.presented_no_changelog_labels)
+ format(CREATE_CHANGELOG_MESSAGE, mr_iid: gitlab.mr_json["iid"], mr_title: sanitized_mr_title, labels: changelog.presented_no_changelog_labels)
end
end
diff --git a/danger/commit_messages/Dangerfile b/danger/commit_messages/Dangerfile
index a7466aa6ffb..d6eb050930c 100644
--- a/danger/commit_messages/Dangerfile
+++ b/danger/commit_messages/Dangerfile
@@ -1,295 +1,162 @@
# frozen_string_literal: true
-require 'json'
+require_relative File.expand_path('../../lib/gitlab/danger/commit_linter', __dir__)
-URL_LIMIT_SUBJECT = "https://chris.beams.io/posts/git-commit/#limit-50"
URL_GIT_COMMIT = "https://chris.beams.io/posts/git-commit/"
-
-# rubocop: disable Style/SignalException
-# rubocop: disable Metrics/CyclomaticComplexity
-# rubocop: disable Metrics/PerceivedComplexity
-
-# Perform various checks against commits. We're not using
-# https://github.com/jonallured/danger-commit_lint because its output is not
-# very helpful, and it doesn't offer the means of ignoring merge commits.
-
-class EmojiChecker
- DIGESTS = File.expand_path('../../fixtures/emojis/digests.json', __dir__)
- ALIASES = File.expand_path('../../fixtures/emojis/aliases.json', __dir__)
-
- # A regex that indicates a piece of text _might_ include an Emoji. The regex
- # alone is not enough, as we'd match `:foo:bar:baz`. Instead, we use this
- # regex to save us from having to check for all possible emoji names when we
- # know one definitely is not included.
- LIKELY_EMOJI = /:[\+a-z0-9_\-]+:/.freeze
-
- def initialize
- names = JSON.parse(File.read(DIGESTS)).keys +
- JSON.parse(File.read(ALIASES)).keys
-
- @emoji = names.map { |name| ":#{name}:" }
- end
-
- def includes_emoji?(text)
- return false unless text.match?(LIKELY_EMOJI)
-
- @emoji.any? { |emoji| text.include?(emoji) }
- end
-end
+MAX_COMMITS_COUNT = 10
def gitlab_danger
@gitlab_danger ||= GitlabDanger.new(helper.gitlab_helper)
end
def fail_commit(commit, message)
- fail("#{commit.sha}: #{message}")
+ self.fail("#{commit.sha}: #{message}")
end
def warn_commit(commit, message)
- warn("#{commit.sha}: #{message}")
-end
-
-def lines_changed_in_commit(commit)
- commit.diff_parent.stats[:total][:lines]
+ self.warn("#{commit.sha}: #{message}")
end
-def subject_starts_with_capital?(subject)
- first_char = subject.chars.first
-
- first_char.upcase == first_char
-end
-
-def too_many_changed_lines?(commit)
- commit.diff_parent.stats[:total][:files] > 3 &&
- lines_changed_in_commit(commit) >= 30
-end
-
-def emoji_checker
- @emoji_checker ||= EmojiChecker.new
+def squash_mr?
+ gitlab_danger.ci? ? gitlab.mr_json['squash'] : false
end
-def unicode_emoji_regex
- @unicode_emoji_regex ||= %r((
- [\u{1F300}-\u{1F5FF}] |
- [\u{1F1E6}-\u{1F1FF}] |
- [\u{2700}-\u{27BF}] |
- [\u{1F900}-\u{1F9FF}] |
- [\u{1F600}-\u{1F64F}] |
- [\u{1F680}-\u{1F6FF}] |
- [\u{2600}-\u{26FF}]
- ))x
+def wip_mr?
+ gitlab_danger.ci? ? gitlab.mr_json['work_in_progress'] : false
end
-def count_filtered_commits(commits)
- commits.count do |commit|
- !commit.message.start_with?('fixup!', 'squash!')
- end
-end
+# Perform various checks against commits. We're not using
+# https://github.com/jonallured/danger-commit_lint because its output is not
+# very helpful, and it doesn't offer the means of ignoring merge commits.
+def lint_commit(commit)
+ linter = Gitlab::Danger::CommitLinter.new(commit)
-def lint_commit(commit) # rubocop:disable Metrics/AbcSize
# For now we'll ignore merge commits, as getting rid of those is a problem
# separate from enforcing good commit messages.
- return false if commit.message.start_with?('Merge branch')
+ return linter if linter.merge?
# We ignore revert commits as they are well structured by Git already
- return false if commit.message.start_with?('Revert "')
+ return linter if linter.revert?
- is_squash = gitlab_danger.ci? ? gitlab.mr_json['squash'] : false
- is_wip = gitlab_danger.ci? ? gitlab.mr_json['work_in_progress'] : false
- is_fixup = commit.message.start_with?('fixup!', 'squash!')
-
- if is_fixup
- # The MR is set to squash - Danger adds an informative notice
- # The MR is not set to squash - Danger fails. if also WIP warn only, not error
- if is_squash
- return false
- end
+ # If MR is set to squash, we ignore fixup commits
+ return linter if linter.fixup? && squash_mr?
- if is_wip
- warn_commit(
- commit,
- 'Squash or Fixup commits must be squashed before merge, or enable squash merge option'
- )
+ if linter.fixup?
+ msg = 'Squash or fixup commits must be squashed before merge, or enable squash merge option'
+ if wip_mr? || squash_mr?
+ warn_commit(commit, msg)
else
- fail_commit(
- commit,
- 'Squash or Fixup commits must be squashed before merge, or enable squash merge option'
- )
+ fail_commit(commit, msg)
end
# Makes no sense to process other rules for fixup commits, they trigger just more noise
- return false
+ return linter
end
# Fail if a suggestion commit is used and squash is not enabled
- if commit.message.start_with?('Apply suggestion to')
- if is_squash
- return false
- else
- fail_commit(
- commit,
- 'If you are applying suggestions, enable squash in the merge request and re-run the failed job'
- )
- return true
+ if linter.suggestion?
+ unless squash_mr?
+ fail_commit(commit, "If you are applying suggestions, enable squash in the merge request and re-run the `danger-review` job")
end
- end
-
- failures = false
- subject, separator, details = commit.message.split("\n", 3)
-
- if subject.split.length < 3
- fail_commit(
- commit,
- 'The commit subject must contain at least three words'
- )
- failures = true
+ return linter
end
- if subject.length > 72
- fail_commit(
- commit,
- 'The commit subject may not be longer than 72 characters'
- )
-
- failures = true
- elsif subject.length > 50
- warn_commit(
- commit,
- "This commit's subject line is acceptable, but please try to [reduce it to 50 characters](#{URL_LIMIT_SUBJECT})."
- )
- end
-
- unless subject_starts_with_capital?(subject)
- fail_commit(commit, 'The commit subject must start with a capital letter')
- failures = true
- end
-
- if subject.end_with?('.')
- fail_commit(commit, 'The commit subject must not end with a period')
- failures = true
- end
-
- if separator && !separator.empty?
- fail_commit(
- commit,
- 'The commit subject and body must be separated by a blank line'
- )
-
- failures = true
- end
-
- details&.each_line do |line|
- line = line.strip
-
- next if line.length <= 72
-
- url_size = line.scan(%r((https?://\S+))).sum { |(url)| url.length }
-
- # If the line includes a URL, we'll allow it to exceed 72 characters, but
- # only if the line _without_ the URL does not exceed this limit.
- next if line.length - url_size <= 72
-
- fail_commit(
- commit,
- 'The commit body should not contain more than 72 characters per line'
- )
+ linter.lint
+end
- failures = true
- end
+def lint_mr_title(mr_title)
+ commit = Struct.new(:message, :sha).new(mr_title)
- if !details && too_many_changed_lines?(commit)
- fail_commit(
- commit,
- 'Commits that change 30 or more lines across at least three files ' \
- 'must describe these changes in the commit body'
- )
+ Gitlab::Danger::CommitLinter.new(commit).lint_subject("merge request title")
+end
- failures = true
- end
+def count_non_fixup_commits(commit_linters)
+ commit_linters.count { |commit_linter| !commit_linter.fixup? }
+end
- if emoji_checker.includes_emoji?(commit.message)
- warn_commit(
- commit,
- 'Avoid the use of Markdown Emoji such as `:+1:`. ' \
- 'These add limited value to the commit message, ' \
- 'and are displayed as plain text outside of GitLab'
- )
+def lint_commits(commits)
+ commit_linters = commits.map { |commit| lint_commit(commit) }
+ failed_commit_linters = commit_linters.select { |commit_linter| commit_linter.failed? }
+ warn_or_fail_commits(failed_commit_linters, default_to_fail: !squash_mr?)
- failures = true
+ if count_non_fixup_commits(commit_linters) > MAX_COMMITS_COUNT
+ level = squash_mr? ? :warn : :fail
+ self.__send__(level, # rubocop:disable GitlabSecurity/PublicSend
+ "This merge request includes more than #{MAX_COMMITS_COUNT} commits. " \
+ 'Please rebase these commits into a smaller number of commits or split ' \
+ 'this merge request into multiple smaller merge requests.')
end
- if commit.message.match?(unicode_emoji_regex)
- fail_commit(
- commit,
- 'Avoid the use of Unicode Emoji. ' \
- 'These add no value to the commit message, ' \
- 'and may not be displayed properly everywhere'
- )
+ if squash_mr?
+ multi_line_commit_linter = commit_linters.detect { |commit_linter| commit_linter.multi_line? }
- failures = true
+ if multi_line_commit_linter && multi_line_commit_linter.lint.failed?
+ warn_or_fail_commits(multi_line_commit_linter)
+ fail_message('The commit message that will be used in the squash commit does not meet our Git commit message standards.')
+ else
+ title_linter = lint_mr_title(gitlab.mr_json['title'])
+ if title_linter.failed?
+ warn_or_fail_commits(title_linter)
+ fail_message('The merge request title that will be used in the squash commit does not meet our Git commit message standards.')
+ end
+ end
+ else
+ if failed_commit_linters.any?
+ fail_message('One or more commit messages do not meet our Git commit message standards.')
+ end
end
+end
- if commit.message.match?(%r(([\w\-\/]+)?(#|!|&|%)\d+\b))
- fail_commit(
- commit,
- 'Use full URLs instead of short references ' \
- '(`gitlab-org/gitlab#123` or `!123`), as short references are ' \
- 'displayed as plain text outside of GitLab'
- )
-
- failures = true
+def warn_or_fail_commits(failed_linters, default_to_fail: true)
+ level = default_to_fail ? :fail : :warn
+
+ Array(failed_linters).each do |linter|
+ linter.problems.each do |problem_key, problem_desc|
+ case problem_key
+ when :subject_above_warning
+ warn_commit(linter.commit, problem_desc)
+ else
+ self.__send__("#{level}_commit", linter.commit, problem_desc) # rubocop:disable GitlabSecurity/PublicSend
+ end
+ end
end
-
- failures
end
-def lint_commits(commits)
- failed = commits.select do |commit|
- lint_commit(commit)
- end
+def fail_message(intro)
+ markdown(<<~MARKDOWN)
+ ## Commit message standards
- if failed.any?
- markdown(<<~MARKDOWN)
- ## Commit message standards
+ #{intro}
- One or more commit messages do not meet our Git commit message standards.
- For more information on how to write a good commit message, take a look at
- [How to Write a Git Commit Message](#{URL_GIT_COMMIT}).
+ For more information on how to write a good commit message, take a look at
+ [How to Write a Git Commit Message](#{URL_GIT_COMMIT}).
- Here is an example of a good commit message:
+ Here is an example of a good commit message:
- Reject ruby interpolation in externalized strings
+ Reject ruby interpolation in externalized strings
- When using ruby interpolation in externalized strings, they can't be
- detected. Which means they will never be presented to be translated.
+ When using ruby interpolation in externalized strings, they can't be
+ detected. Which means they will never be presented to be translated.
- To mix variables into translations we need to use `sprintf`
- instead.
+ To mix variables into translations we need to use `sprintf`
+ instead.
- Instead of:
+ Instead of:
- _("Hello \#{subject}")
+ _("Hello \#{subject}")
- Use:
+ Use:
- _("Hello %{subject}") % { subject: 'world' }
+ _("Hello %{subject}") % { subject: 'world' }
- This is an example of a bad commit message:
+ This is an example of a bad commit message:
- updated README.md
+ updated README.md
- This commit message is bad because although it tells us that README.md is
- updated, it doesn't tell us why or how it was updated.
- MARKDOWN
- end
+ This commit message is bad because although it tells us that README.md is
+ updated, it doesn't tell us why or how it was updated.
+ MARKDOWN
end
lint_commits(git.commits)
-
-if count_filtered_commits(git.commits) > 10
- fail(
- 'This merge request includes more than 10 commits. ' \
- 'Please rebase these commits into a smaller number of commits.'
- )
-end