diff options
Diffstat (limited to 'danger')
-rw-r--r-- | danger/changelog/Dangerfile | 8 | ||||
-rw-r--r-- | danger/commit_messages/Dangerfile | 325 |
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 |