Skip to content

G
gitlab-ce
Commit 16ba3dd5 authored by Albert Salim's avatar Albert Salim
Browse files
Options

Merge branch 'try-out-the-commit-messages-rule-from-gitlab-dangerfiles' into 'master' 

Use the 'commit_messages' rule from gitlab-dangerfiles

See merge request gitlab-org/gitlab!62914
parents a7b6430b e75c5a7b
Hide whitespace changes
Inline Side-by-side
Showing with 8 additions and 156 deletions
Gemfile
View file @ 16ba3dd5
......@@ -402,7 +402,7 @@ group :development, :test do
end
group :development, :test, :danger do
gem 'gitlab-dangerfiles', '~> 2.0.0', require: false
gem 'gitlab-dangerfiles', '~> 2.1.2', require: false
end
group :development, :test, :coverage do
......
Gemfile.lock
View file @ 16ba3dd5
......@@ -451,7 +451,7 @@ GEM
terminal-table (~> 1.5, >= 1.5.1)
gitlab-chronic (0.10.5)
numerizer (~> 0.2)
gitlab-dangerfiles (2.0.0)
gitlab-dangerfiles (2.1.2)
danger-gitlab
gitlab-experiment (0.5.4)
activesupport (>= 3.0)
......@@ -1474,7 +1474,7 @@ DEPENDENCIES
gitaly (~> 13.12.0.pre.rc1)
github-markup (~> 1.7.0)
gitlab-chronic (~> 0.10.5)
gitlab-dangerfiles (~> 2.0.0)
gitlab-dangerfiles (~> 2.1.2)
gitlab-experiment (~> 0.5.4)
gitlab-fog-azure-rm (~> 1.1.1)
gitlab-fog-google (~> 1.13)
......
danger/commit_messages/Dangerfile deleted 100644 → 0
View file @ a7b6430b
# frozen_string_literal: true
require 'gitlab/dangerfiles/commit_linter'
require 'gitlab/dangerfiles/merge_request_linter'
COMMIT_MESSAGE_GUIDELINES = "https://docs.gitlab.com/ee/development/contributing/merge_request_workflow.html#commit-messages-guidelines"
MORE_INFO = "For more information, take a look at our [Commit message guidelines](#{COMMIT_MESSAGE_GUIDELINES})."
THE_DANGER_JOB_TEXT = "the `danger-review` job"
MAX_COMMITS_COUNT = 10
MAX_COMMITS_COUNT_EXCEEDED_MESSAGE = <<~MSG
This merge request includes more than %<max_commits_count>d commits. Each commit should meet the following criteria:
1. Have a well-written commit message.
1. Has all tests passing when used on its own (e.g. when using git checkout SHA).
1. Can be reverted on its own without also requiring the revert of commit that came before it.
1. Is small enough that it can be reviewed in isolation in under 30 minutes or so.
If this merge request contains commits that do not meet this criteria and/or contains intermediate work, please rebase these commits into a smaller number of commits or split this merge request into multiple smaller merge requests.
MSG
def fail_commit(commit, message, more_info: true)
self.fail(build_message(commit, message, more_info: more_info))
end
def warn_commit(commit, message, more_info: true)
self.warn(build_message(commit, message, more_info: more_info))
end
def build_message(commit, message, more_info: true)
[message].tap do |full_message|
full_message << ". #{MORE_INFO}" if more_info
full_message.unshift("#{commit.sha}: ") if commit.sha
end.join
end
def squash_mr?
# Locally, we assume the MR is set to be squashed so that the user only sees warnings instead of errors.
helper.ci? ? gitlab.mr_json['squash'] : true
end
def wip_mr?
helper.ci? ? gitlab.mr_json['work_in_progress'] : false
end
def danger_job_link
helper.ci? ? "[#{THE_DANGER_JOB_TEXT}](#{ENV['CI_JOB_URL']})" : THE_DANGER_JOB_TEXT
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::Dangerfiles::CommitLinter.new(commit)
# For now we'll ignore merge commits, as getting rid of those is a problem
# separate from enforcing good commit messages.
return linter if linter.merge?
# We ignore revert commits as they are well structured by Git already
return linter if linter.revert?
# If MR is set to squash, we ignore fixup commits
return linter if linter.fixup? && squash_mr?
if linter.fixup?
msg = "Squash or fixup commits must be squashed before merge, or enable squash merge option and re-run #{danger_job_link}."
if wip_mr? || squash_mr?
warn_commit(commit, msg, more_info: false)
else
fail_commit(commit, msg, more_info: false)
end
# Makes no sense to process other rules for fixup commits, they trigger just more noise
return linter
end
# Fail if a suggestion commit is used and squash is not enabled
if linter.suggestion?
unless squash_mr?
fail_commit(commit, "If you are applying suggestions, enable squash in the merge request and re-run #{danger_job_link}.", more_info: false)
end
return linter
end
linter.lint
end
def lint_mr_title(mr_title)
commit = Struct.new(:message, :sha).new(mr_title)
Gitlab::Dangerfiles::MergeRequestLinter.new(commit).lint
end
def count_non_fixup_commits(commit_linters)
commit_linters.count { |commit_linter| !commit_linter.fixup? }
end
def lint_commits(commits)
commit_linters = commits.map { |commit| lint_commit(commit) }
if squash_mr?
multi_line_commit_linter = commit_linters.detect { |commit_linter| !commit_linter.merge? && commit_linter.multi_line? }
if multi_line_commit_linter && multi_line_commit_linter.failed?
warn_or_fail_commits(multi_line_commit_linter)
commit_linters.delete(multi_line_commit_linter) # Don't show an error (here) and a warning (below)
elsif helper.ci? # We don't have access to the MR title locally
title_linter = lint_mr_title(gitlab.mr_json['title'])
if title_linter.failed?
warn_or_fail_commits(title_linter)
end
end
else
if count_non_fixup_commits(commit_linters) > MAX_COMMITS_COUNT
self.warn(format(MAX_COMMITS_COUNT_EXCEEDED_MESSAGE, max_commits_count: MAX_COMMITS_COUNT))
end
end
failed_commit_linters = commit_linters.select { |commit_linter| commit_linter.failed? }
warn_or_fail_commits(failed_commit_linters, default_to_fail: !squash_mr?)
end
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_too_short, :subject_above_warning, :details_too_many_changes, :details_line_too_long
warn_commit(linter.commit, problem_desc)
else
self.__send__("#{level}_commit", linter.commit, problem_desc) # rubocop:disable GitlabSecurity/PublicSend
end
end
end
end
# As part of https://gitlab.com/groups/gitlab-org/-/epics/4826 we are
# vendoring workhorse commits from the stand-alone gitlab-workhorse
# repo. There is no point in linting commits that we want to vendor as
# is.
def workhorse_changes?
git.diff.any? { |file| file.path.start_with?('workhorse/') }
end
lint_commits(git.commits) unless workhorse_changes?
spec/tooling/danger/project_helper_spec.rb
View file @ 16ba3dd5
......@@ -112,10 +112,10 @@ RSpec.describe Tooling::Danger::ProjectHelper do
'FOO_VERSION' | [:backend]
'Dangerfile' | [:engineering_productivity]
'danger/commit_messages/Dangerfile' | [:engineering_productivity]
'ee/danger/commit_messages/Dangerfile' | [:engineering_productivity]
'danger/commit_messages/' | [:engineering_productivity]
'ee/danger/commit_messages/' | [:engineering_productivity]
'danger/bundle_size/Dangerfile' | [:engineering_productivity]
'ee/danger/bundle_size/Dangerfile' | [:engineering_productivity]
'danger/bundle_size/' | [:engineering_productivity]
'ee/danger/bundle_size/' | [:engineering_productivity]
'.gitlab-ci.yml' | [:engineering_productivity]
'.gitlab/ci/cng.gitlab-ci.yml' | [:engineering_productivity]
'.gitlab/ci/ee-specific-checks.gitlab-ci.yml' | [:engineering_productivity]
......@@ -218,7 +218,7 @@ RSpec.describe Tooling::Danger::ProjectHelper do
describe '.local_warning_message' do
it 'returns an informational message with rules that can run' do
expect(described_class.local_warning_message).to eq('==> Only the following Danger rules can be run locally: changelog, commit_messages, database, datateam, documentation, duplicate_yarn_dependencies, eslint, karma, pajamas, pipeline, prettier, product_intelligence, utility_css')
expect(described_class.local_warning_message).to eq('==> Only the following Danger rules can be run locally: changelog, database, datateam, documentation, duplicate_yarn_dependencies, eslint, karma, pajamas, pipeline, prettier, product_intelligence, utility_css')
end
end
......
tooling/danger/project_helper.rb
View file @ 16ba3dd5
......@@ -5,7 +5,6 @@ module Tooling
module ProjectHelper
LOCAL_RULES ||= %w[
changelog
commit_messages
database
datateam
documentation
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7