From 59d5f1b34b23fd573ce1d506cc8e98ab34a61c57 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?R=C3=A9my=20Coutable?= <remy@rymai.me>
Date: Tue, 22 Feb 2022 16:44:27 +0100
Subject: [PATCH] danger: Use changelog rule from gitlab-dangerfiles
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Changelog: other
Signed-off-by: Rémy Coutable <remy@rymai.me>
---
Dangerfile | 21 -
Gemfile | 2 +-
Gemfile.lock | 6 +-
danger/changelog/Dangerfile | 3 -
danger/database/Dangerfile | 4 +-
danger/feature_flag/Dangerfile | 2 +-
danger/plugins/changelog.rb | 10 -
danger/product_intelligence/Dangerfile | 2 +-
danger/specialization_labels/Dangerfile | 2 +-
danger/z_metadata/Dangerfile | 14 -
doc/development/dangerbot.md | 36 +-
lefthook.yml | 2 +-
spec/tooling/danger/changelog_spec.rb | 467 ---------------------
spec/tooling/danger/project_helper_spec.rb | 14 +-
tooling/danger/changelog.rb | 232 ----------
tooling/danger/project_helper.rb | 9 -
16 files changed, 39 insertions(+), 787 deletions(-)
delete mode 100644 danger/changelog/Dangerfile
delete mode 100644 danger/plugins/changelog.rb
delete mode 100644 spec/tooling/danger/changelog_spec.rb
delete mode 100644 tooling/danger/changelog.rb
diff --git a/Dangerfile b/Dangerfile
index 1fc2005498d9d..aaa1aae813bd7 100644
--- a/Dangerfile
+++ b/Dangerfile
@@ -24,24 +24,3 @@ return if helper.release_automation?
project_helper.rule_names.each do |rule|
danger.import_dangerfile(path: File.join('danger', rule))
end
-
-anything_to_post = status_report.values.any? { |data| data.any? }
-
-return unless helper.ci?
-
-def post_labels
- gitlab.api.update_merge_request(gitlab.mr_json['project_id'],
- gitlab.mr_json['iid'],
- add_labels: project_helper.labels_to_add.join(','))
-rescue Gitlab::Error::Forbidden
- labels = project_helper.labels_to_add.map { |label| %Q(~"#{label}") }
- warn("This Merge Request needs to be labelled with #{labels.join(' ')}. Please request a reviewer or maintainer to add them.")
-end
-
-if project_helper.labels_to_add.any?
- post_labels
-end
-
-if anything_to_post
- markdown("**If needed, you can retry the [🔠`danger-review` job](#{ENV['CI_JOB_URL']}) that generated this comment.**")
-end
diff --git a/Gemfile b/Gemfile
index d5879e7d47de7..6d8f2dde4598d 100644
--- a/Gemfile
+++ b/Gemfile
@@ -398,7 +398,7 @@ group :development, :test do
end
group :development, :test, :danger do
- gem 'gitlab-dangerfiles', '~> 2.8.0', require: false
+ gem 'gitlab-dangerfiles', '~> 2.9.3', require: false
end
group :development, :test, :coverage do
diff --git a/Gemfile.lock b/Gemfile.lock
index 5e98b5d2981af..f0063f0dee746 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -221,7 +221,7 @@ GEM
css_parser (1.7.0)
addressable
daemons (1.3.1)
- danger (8.4.2)
+ danger (8.4.3)
claide (~> 1.0)
claide-plugins (>= 0.9.2)
colored2 (~> 3.1)
@@ -460,7 +460,7 @@ GEM
terminal-table (~> 1.5, >= 1.5.1)
gitlab-chronic (0.10.5)
numerizer (~> 0.2)
- gitlab-dangerfiles (2.8.0)
+ gitlab-dangerfiles (2.9.3)
danger (>= 8.3.1)
danger-gitlab (>= 8.0.0)
gitlab-experiment (0.7.0)
@@ -1470,7 +1470,7 @@ DEPENDENCIES
gitaly (~> 14.8.0.pre.rc1)
github-markup (~> 1.7.0)
gitlab-chronic (~> 0.10.5)
- gitlab-dangerfiles (~> 2.8.0)
+ gitlab-dangerfiles (~> 2.9.3)
gitlab-experiment (~> 0.7.0)
gitlab-fog-azure-rm (~> 1.2.0)
gitlab-labkit (~> 0.22.0)
diff --git a/danger/changelog/Dangerfile b/danger/changelog/Dangerfile
deleted file mode 100644
index 83c6f68869b41..0000000000000
--- a/danger/changelog/Dangerfile
+++ /dev/null
@@ -1,3 +0,0 @@
-# frozen_string_literal: true
-
-changelog.check!
diff --git a/danger/database/Dangerfile b/danger/database/Dangerfile
index b4e06c21fe4f7..0128f0fa19513 100644
--- a/danger/database/Dangerfile
+++ b/danger/database/Dangerfile
@@ -65,7 +65,7 @@ if gitlab.mr_labels.include?('database') || db_paths_to_review.any?
markdown(DB_REMOVE_MESSAGE)
end
- unless helper.has_database_scoped_labels?
- project_helper.labels_to_add << 'database::review pending'
+ unless helper.has_scoped_label_with_scope?("database")
+ helper.labels_to_add << 'database::review pending'
end
end
diff --git a/danger/feature_flag/Dangerfile b/danger/feature_flag/Dangerfile
index d6c1c53cddcb0..5fe9d42a7a1c4 100644
--- a/danger/feature_flag/Dangerfile
+++ b/danger/feature_flag/Dangerfile
@@ -58,7 +58,7 @@ def message_for_feature_flag_with_group!(feature_flag:, mr_group_label:)
return if feature_flag.group_match_mr_label?(mr_group_label)
if mr_group_label.nil?
- project_helper.labels_to_add << feature_flag.group
+ helper.labels_to_add << feature_flag.group
else
fail %(`group` is set to ~"#{feature_flag.group}" in #{gitlab.html_link(feature_flag.path)}, which does not match ~"#{mr_group_label}" set on the MR!)
end
diff --git a/danger/plugins/changelog.rb b/danger/plugins/changelog.rb
deleted file mode 100644
index 02ff405c410a9..0000000000000
--- a/danger/plugins/changelog.rb
+++ /dev/null
@@ -1,10 +0,0 @@
-# frozen_string_literal: true
-
-require_relative '../../tooling/danger/changelog'
-
-module Danger
- class Changelog < ::Danger::Plugin
- # Put the helper code somewhere it can be tested
- include Tooling::Danger::Changelog
- end
-end
diff --git a/danger/product_intelligence/Dangerfile b/danger/product_intelligence/Dangerfile
index 01a2f9b6febd6..77d714a7f600f 100644
--- a/danger/product_intelligence/Dangerfile
+++ b/danger/product_intelligence/Dangerfile
@@ -19,4 +19,4 @@ return if product_intelligence_paths_to_review.empty? || product_intelligence.sk
warn format(CHANGED_FILES_MESSAGE, changed_files: helper.markdown_list(product_intelligence_paths_to_review)) unless product_intelligence.has_approved_label?
-project_helper.labels_to_add.concat(labels_to_add) unless labels_to_add.empty?
+helper.labels_to_add.merge(labels_to_add) unless labels_to_add.empty?
diff --git a/danger/specialization_labels/Dangerfile b/danger/specialization_labels/Dangerfile
index cb4c8c96f4f8c..7d1c83697fd0d 100644
--- a/danger/specialization_labels/Dangerfile
+++ b/danger/specialization_labels/Dangerfile
@@ -26,4 +26,4 @@ labels_to_add = helper.changes_by_category.each_with_object([]) do |(category, _
memo << label
end
-project_helper.labels_to_add.concat(labels_to_add) if labels_to_add.any?
+helper.labels_to_add.merge(labels_to_add) if labels_to_add.any?
diff --git a/danger/z_metadata/Dangerfile b/danger/z_metadata/Dangerfile
index 0a70554486f77..546fdc8de5f87 100644
--- a/danger/z_metadata/Dangerfile
+++ b/danger/z_metadata/Dangerfile
@@ -4,24 +4,10 @@
DEFAULT_BRANCH = 'master'
-TYPE_LABELS = [
- 'type::feature',
- 'feature::addition',
- 'type::maintenance',
- 'type::tooling',
- 'tooling::pipelines',
- 'tooling::workflow',
- 'type::bug'
-].freeze
-
if gitlab.mr_body.size < 5
fail "Please provide a proper merge request description."
end
-if (TYPE_LABELS & (gitlab.mr_labels + project_helper.labels_to_add)).empty?
- warn 'Please add a [merge request type](https://about.gitlab.com/handbook/engineering/metrics/#work-type-classification) to this merge request.'
-end
-
unless gitlab.mr_json["assignee"]
warn "This merge request does not have any assignee yet. Setting an assignee clarifies who needs to take action on the merge request at any given time."
end
diff --git a/doc/development/dangerbot.md b/doc/development/dangerbot.md
index 8da1f5700e53f..e2ff56351a7ba 100644
--- a/doc/development/dangerbot.md
+++ b/doc/development/dangerbot.md
@@ -121,12 +121,13 @@ to revert the change before merging!
#### Adding labels via Danger
NOTE:
-This is currently applicable to the [`gitlab-org/gitlab`](https://gitlab.com/gitlab-org/gitlab)
-project only.
+This is applicable to all the projects that use the [`gitlab-dangerfiles` gem](https://rubygems.org/gems/gitlab-dangerfiles).
Danger is often used to improve MR hygiene by adding labels. Instead of calling the
-API directly in your `Dangerfile`, add the labels to the `project_helper.labels_to_add` array.
-The main `Dangerfile` will then take care of adding the labels to the MR with a single API call.
+API directly in your `Dangerfile`, add the labels to `helper.labels_to_add` set (with `helper.labels_to_add << label`
+or `helper.labels_to_add.merge(array_of_labels)`.
+`gitlab-dangerfiles` will then take care of adding the labels to the MR with a single API call after all the rules
+have had the chance to add to `helper.labels_to_add`.
#### Shared rules and plugins
@@ -135,11 +136,30 @@ upstreaming them to the [`gitlab-dangerfiles`](https://gitlab.com/gitlab-org/rub
#### Enable Danger on a project
-To enable the Dangerfile on another existing GitLab project, run the following
-extra steps:
+To enable the Dangerfile on another existing GitLab project, complete the following steps:
-1. Create a [Project access tokens](../user/project/settings/project_access_tokens.md).
-1. Add the token as a CI/CD project variable named `DANGER_GITLAB_API_TOKEN`.
+1. Add [`gitlab-dangerfiles`](https://rubygems.org/gems/gitlab-dangerfiles) to your `Gemfile`.
+1. Create a `Dangerfile` with the following content:
+
+ ```ruby
+ require_relative "lib/gitlab-dangerfiles"
+
+ Gitlab::Dangerfiles.for_project(self, &:import_defaults)
+ ```
+
+1. Add the following to your CI/CD configuration:
+
+ ```yaml
+ include:
+ - project: 'gitlab-org/quality/pipeline-common'
+ file:
+ - '/ci/danger-review.yml'
+ ```
+
+1. If your project is in the `gitlab-org` group, you don't need to set up any token as the `DANGER_GITLAB_API_TOKEN`
+ variable is available at the group level. If not, follow these last steps:
+ 1. Create a [Project access tokens](../user/project/settings/project_access_tokens.md).
+ 1. Add the token as a CI/CD project variable named `DANGER_GITLAB_API_TOKEN`.
You should add the ~"Danger bot" label to the merge request before sending it
for review.
diff --git a/lefthook.yml b/lefthook.yml
index f2b020453689c..62bbb440b296f 100644
--- a/lefthook.yml
+++ b/lefthook.yml
@@ -2,7 +2,7 @@ pre-push:
parallel: true
commands:
danger:
- run: bundle exec danger dry_run
+ run: CI_PROJECT_DIR=. bundle exec danger dry_run
eslint:
tags: frontend style
files: git diff --name-only --diff-filter=d $(git merge-base origin/master HEAD)..HEAD
diff --git a/spec/tooling/danger/changelog_spec.rb b/spec/tooling/danger/changelog_spec.rb
deleted file mode 100644
index 377c3e881c9e2..0000000000000
--- a/spec/tooling/danger/changelog_spec.rb
+++ /dev/null
@@ -1,467 +0,0 @@
-# frozen_string_literal: true
-
-require 'gitlab-dangerfiles'
-require 'gitlab/dangerfiles/spec_helper'
-
-require_relative '../../../tooling/danger/changelog'
-require_relative '../../../tooling/danger/project_helper'
-
-RSpec.describe Tooling::Danger::Changelog do
- include_context "with dangerfile"
-
- let(:fake_danger) { DangerSpecHelper.fake_danger.include(described_class) }
- let(:fake_project_helper) { double('fake-project-helper', helper: fake_helper).tap { |h| h.class.include(Tooling::Danger::ProjectHelper) } }
-
- subject(:changelog) { fake_danger.new(helper: fake_helper) }
-
- before do
- allow(changelog).to receive(:project_helper).and_return(fake_project_helper)
- end
-
- describe '#check_changelog_commit_categories' do
- context 'when all changelog commits are correct' do
- it 'does not produce any messages' do
- commit = double(:commit, message: "foo\nChangelog: fixed")
-
- allow(changelog).to receive(:changelog_commits).and_return([commit])
-
- expect(changelog).not_to receive(:fail)
-
- changelog.check_changelog_commit_categories
- end
- end
-
- context 'when a commit has an incorrect trailer' do
- it 'adds a message' do
- commit = double(:commit, message: "foo\nChangelog: foo", sha: '123')
-
- allow(changelog).to receive(:changelog_commits).and_return([commit])
-
- expect(changelog).to receive(:fail)
-
- changelog.check_changelog_commit_categories
- end
- end
- end
-
- describe '#check_changelog_trailer' do
- subject { changelog.check_changelog_trailer(commit) }
-
- context "when commit include a changelog trailer with an unknown category" do
- let(:commit) { double('commit', message: "Hello world\n\nChangelog: foo", sha: "abc123") }
-
- it { is_expected.to have_attributes(errors: ["Commit #{commit.sha} uses an invalid changelog category: foo"]) }
- end
-
- context 'when a commit uses the wrong casing for a trailer' do
- let(:commit) { double('commit', message: "Hello world\n\nchangelog: foo", sha: "abc123") }
-
- it { is_expected.to have_attributes(errors: ["The changelog trailer for commit #{commit.sha} must be `Changelog` (starting with a capital C), not `changelog`"]) }
- end
-
- described_class::CATEGORIES.each do |category|
- context "when commit include a changelog trailer with category set to '#{category}'" do
- let(:commit) { double('commit', message: "Hello world\n\nChangelog: #{category}", sha: "abc123") }
-
- it { is_expected.to have_attributes(errors: []) }
- end
- end
- end
-
- describe '#check_changelog_path' do
- let(:changelog_path) { 'changelog-path.yml' }
- let(:foss_change) { nil }
- let(:ee_change) { nil }
- let(:changelog_change) { nil }
- let(:changes) { changes_class.new([foss_change, ee_change, changelog_change].compact) }
-
- before do
- allow(changelog).to receive(:present?).and_return(true)
- end
-
- subject { changelog.check_changelog_path }
-
- context "when changelog is not present" do
- before do
- allow(changelog).to receive(:present?).and_return(false)
- end
-
- it { is_expected.to have_attributes(errors: [], warnings: [], markdowns: [], messages: []) }
- end
-
- context "with EE changes" do
- let(:ee_change) { change_class.new('ee/app/models/foo.rb', :added, :backend) }
-
- context "and a non-EE changelog, and changelog not required" do
- before do
- allow(changelog).to receive(:required?).and_return(false)
- allow(changelog).to receive(:ee_changelog?).and_return(false)
- end
-
- it { is_expected.to have_attributes(warnings: ["This MR changes code in `ee/`, but its Changelog commit is missing the [`EE: true` trailer](https://docs.gitlab.com/ee/development/changelog.html#gitlab-enterprise-changes). Consider adding it to your Changelog commits."]) }
- end
-
- context "and a EE changelog" do
- before do
- allow(changelog).to receive(:ee_changelog?).and_return(true)
- end
-
- it { is_expected.to have_attributes(errors: [], warnings: [], markdowns: [], messages: []) }
-
- context "and there are DB changes" do
- let(:foss_change) { change_class.new('db/migrate/foo.rb', :added, :migration) }
-
- it { is_expected.to have_attributes(warnings: ["This MR has a Changelog commit with the `EE: true` trailer, but there are database changes which [requires](https://docs.gitlab.com/ee/development/changelog.html#what-warrants-a-changelog-entry) the Changelog commit to not have the `EE: true` trailer. Consider removing the `EE: true` trailer from your commits."]) }
- end
- end
- end
-
- context "with no EE changes" do
- let(:foss_change) { change_class.new('app/models/foo.rb', :added, :backend) }
-
- context "and a non-EE changelog" do
- before do
- allow(changelog).to receive(:ee_changelog?).and_return(false)
- end
-
- it { is_expected.to have_attributes(errors: [], warnings: [], markdowns: [], messages: []) }
- end
-
- context "and a EE changelog" do
- before do
- allow(changelog).to receive(:ee_changelog?).and_return(true)
- end
-
- it { is_expected.to have_attributes(warnings: ["This MR has a Changelog commit for EE, but no code changes in `ee/`. Consider removing the `EE: true` trailer from your commits."]) }
- end
- end
- end
-
- describe '#required_reasons' do
- subject { changelog.required_reasons }
-
- context "added files contain a migration" do
- let(:changes) { changes_class.new([change_class.new('foo', :added, :migration)]) }
-
- it { is_expected.to include(:db_changes) }
- end
-
- context "removed files contains a feature flag" do
- let(:changes) { changes_class.new([change_class.new('foo', :deleted, :feature_flag)]) }
-
- it { is_expected.to include(:feature_flag_removed) }
- end
-
- context "added files do not contain a migration" do
- let(:changes) { changes_class.new([change_class.new('foo', :added, :frontend)]) }
-
- it { is_expected.to be_empty }
- end
-
- context "removed files do not contain a feature flag" do
- let(:changes) { changes_class.new([change_class.new('foo', :deleted, :backend)]) }
-
- it { is_expected.to be_empty }
- end
- end
-
- describe '#required?' do
- subject { changelog.required? }
-
- context 'added files contain a migration' do
- let(:changes) { changes_class.new([change_class.new('foo', :added, :migration)]) }
-
- it { is_expected.to be_truthy }
- end
-
- context "removed files contains a feature flag" do
- let(:changes) { changes_class.new([change_class.new('foo', :deleted, :feature_flag)]) }
-
- it { is_expected.to be_truthy }
- end
-
- context 'added files do not contain a migration' do
- let(:changes) { changes_class.new([change_class.new('foo', :added, :frontend)]) }
-
- it { is_expected.to be_falsey }
- end
-
- context "removed files do not contain a feature flag" do
- let(:changes) { changes_class.new([change_class.new('foo', :deleted, :backend)]) }
-
- it { is_expected.to be_falsey }
- end
- end
-
- describe '#optional?' do
- let(:category_with_changelog) { :backend }
- let(:label_with_changelog) { 'frontend' }
- let(:category_without_changelog) { Tooling::Danger::Changelog::NO_CHANGELOG_CATEGORIES.first }
- let(:label_without_changelog) { Tooling::Danger::Changelog::NO_CHANGELOG_LABELS.first }
-
- subject { changelog.optional? }
-
- context 'when MR contains only categories requiring no changelog' do
- let(:changes) { changes_class.new([change_class.new('foo', :modified, category_without_changelog)]) }
-
- it 'is falsey' do
- is_expected.to be_falsy
- end
- end
-
- context 'when MR contains a label that require no changelog' do
- let(:changes) { changes_class.new([change_class.new('foo', :modified, category_with_changelog)]) }
- let(:mr_labels) { [label_with_changelog, label_without_changelog] }
-
- it 'is falsey' do
- is_expected.to be_falsy
- end
- end
-
- context 'when MR contains a category that require changelog and a category that require no changelog' do
- let(:changes) { changes_class.new([change_class.new('foo', :modified, category_with_changelog), change_class.new('foo', :modified, category_without_changelog)]) }
-
- context 'with no labels' do
- it 'is truthy' do
- is_expected.to be_truthy
- end
- end
-
- context 'with changelog label' do
- let(:mr_labels) { ['type::feature'] }
-
- it 'is truthy' do
- is_expected.to be_truthy
- end
- end
-
- context 'with no changelog label' do
- let(:mr_labels) { ['type::tooling'] }
-
- it 'is truthy' do
- is_expected.to be_falsey
- end
- end
- end
- end
-
- describe '#present?' do
- it 'returns true when a Changelog commit is present' do
- allow(changelog)
- .to receive(:valid_changelog_commits)
- .and_return([double(:commit)])
-
- expect(changelog).to be_present
- end
-
- it 'returns false when a Changelog commit is missing' do
- allow(changelog).to receive(:valid_changelog_commits).and_return([])
-
- expect(changelog).not_to be_present
- end
- end
-
- describe '#changelog_commits' do
- it 'returns the commits that include a Changelog trailer' do
- commit1 = double(:commit, message: "foo\nChangelog: fixed")
- commit2 = double(:commit, message: "bar\nChangelog: kittens")
- commit3 = double(:commit, message: 'testing')
- git = double(:git)
-
- allow(changelog).to receive(:git).and_return(git)
- allow(git).to receive(:commits).and_return([commit1, commit2, commit3])
-
- expect(changelog.changelog_commits).to eq([commit1, commit2])
- end
- end
-
- describe '#valid_changelog_commits' do
- it 'returns the commits with a valid Changelog trailer' do
- commit1 = double(:commit, message: "foo\nChangelog: fixed")
- commit2 = double(:commit, message: "bar\nChangelog: kittens")
-
- allow(changelog)
- .to receive(:changelog_commits)
- .and_return([commit1, commit2])
-
- expect(changelog.valid_changelog_commits).to eq([commit1])
- end
- end
-
- describe '#ee_changelog?' do
- it 'returns true when an EE changelog commit is present' do
- commit = double(:commit, message: "foo\nEE: true")
-
- allow(changelog).to receive(:changelog_commits).and_return([commit])
-
- expect(changelog.ee_changelog?).to eq(true)
- end
-
- it 'returns false when an EE changelog commit is missing' do
- commit = double(:commit, message: 'foo')
-
- allow(changelog).to receive(:changelog_commits).and_return([commit])
-
- expect(changelog.ee_changelog?).to eq(false)
- end
- end
-
- describe '#modified_text' do
- subject { changelog.modified_text }
-
- context 'when in CI context' do
- shared_examples 'changelog modified text' do |key|
- specify do
- expect(subject).to include('CHANGELOG.md was edited')
- expect(subject).to include('`Changelog` trailer')
- expect(subject).to include('`EE: true`')
- end
- end
-
- before do
- allow(fake_helper).to receive(:ci?).and_return(true)
- end
-
- context "when title is not changed from sanitization", :aggregate_failures do
- let(:mr_title) { 'Fake Title' }
-
- it_behaves_like 'changelog modified text'
- end
-
- context "when title needs sanitization", :aggregate_failures do
- let(:mr_title) { 'DRAFT: Fake Title' }
-
- it_behaves_like 'changelog modified text'
- end
- end
-
- context 'when in local context' do
- let(:mr_title) { 'Fake Title' }
-
- before do
- allow(fake_helper).to receive(:ci?).and_return(false)
- end
-
- specify do
- expect(subject).to include('CHANGELOG.md was edited')
- expect(subject).not_to include('`Changelog` trailer')
- end
- end
- end
-
- describe '#required_texts' do
- let(:mr_title) { 'Fake Title' }
-
- subject { changelog.required_texts }
-
- context 'when in CI context' do
- before do
- allow(fake_helper).to receive(:ci?).and_return(true)
- end
-
- shared_examples 'changelog required text' do |key|
- specify do
- expect(subject).to have_key(key)
- expect(subject[key]).to include('CHANGELOG missing')
- expect(subject[key]).to include('`Changelog` trailer')
- end
- end
-
- context 'with a new migration file' do
- let(:changes) { changes_class.new([change_class.new('foo', :added, :migration)]) }
-
- context "when title is not changed from sanitization", :aggregate_failures do
- it_behaves_like 'changelog required text', :db_changes
- end
-
- context "when title needs sanitization", :aggregate_failures do
- let(:mr_title) { 'DRAFT: Fake Title' }
-
- it_behaves_like 'changelog required text', :db_changes
- end
- end
-
- context 'with a removed feature flag file' do
- let(:changes) { changes_class.new([change_class.new('foo', :deleted, :feature_flag)]) }
-
- it_behaves_like 'changelog required text', :feature_flag_removed
- end
- end
-
- context 'when in local context' do
- before do
- allow(fake_helper).to receive(:ci?).and_return(false)
- end
-
- shared_examples 'changelog required text' do |key|
- specify do
- expect(subject).to have_key(key)
- expect(subject[key]).to include('CHANGELOG missing')
- expect(subject[key]).not_to include('`Changelog` trailer')
- end
- end
-
- context 'with a new migration file' do
- let(:changes) { changes_class.new([change_class.new('foo', :added, :migration)]) }
-
- context "when title is not changed from sanitization", :aggregate_failures do
- it_behaves_like 'changelog required text', :db_changes
- end
-
- context "when title needs sanitization", :aggregate_failures do
- let(:mr_title) { 'DRAFT: Fake Title' }
-
- it_behaves_like 'changelog required text', :db_changes
- end
- end
-
- context 'with a removed feature flag file' do
- let(:changes) { changes_class.new([change_class.new('foo', :deleted, :feature_flag)]) }
-
- it_behaves_like 'changelog required text', :feature_flag_removed
- end
- end
- end
-
- describe '#optional_text' do
- subject { changelog.optional_text }
-
- context 'when in CI context' do
- shared_examples 'changelog optional text' do |key|
- specify do
- expect(subject).to include('CHANGELOG missing')
- expect(subject).to include('`Changelog` trailer')
- expect(subject).to include('EE: true')
- end
- end
-
- before do
- allow(fake_helper).to receive(:ci?).and_return(true)
- end
-
- context "when title is not changed from sanitization", :aggregate_failures do
- let(:mr_title) { 'Fake Title' }
-
- it_behaves_like 'changelog optional text'
- end
-
- context "when title needs sanitization", :aggregate_failures do
- let(:mr_title) { 'DRAFT: Fake Title' }
-
- it_behaves_like 'changelog optional text'
- end
- end
-
- context 'when in local context' do
- let(:mr_title) { 'Fake Title' }
-
- before do
- allow(fake_helper).to receive(:ci?).and_return(false)
- end
-
- specify do
- expect(subject).to include('CHANGELOG missing')
- end
- end
- end
-end
diff --git a/spec/tooling/danger/project_helper_spec.rb b/spec/tooling/danger/project_helper_spec.rb
index eaafb13b61a50..679fa336c8799 100644
--- a/spec/tooling/danger/project_helper_spec.rb
+++ b/spec/tooling/danger/project_helper_spec.rb
@@ -274,7 +274,7 @@
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, ci_config, database, documentation, duplicate_yarn_dependencies, eslint, gitaly, pajamas, pipeline, prettier, product_intelligence, utility_css, vue_shared_documentation, datateam')
+ expect(described_class.local_warning_message).to eq('==> Only the following Danger rules can be run locally: ci_config, database, documentation, duplicate_yarn_dependencies, eslint, gitaly, pajamas, pipeline, prettier, product_intelligence, utility_css, vue_shared_documentation, datateam')
end
end
@@ -306,18 +306,6 @@
end
end
- describe '#all_ee_changes' do
- subject { project_helper.all_ee_changes }
-
- it 'returns all changed files starting with ee/' do
- changes = double
- expect(fake_helper).to receive(:changes).and_return(changes)
- expect(changes).to receive(:files).and_return(%w[fr/ee/beer.rb ee/wine.rb ee/lib/ido.rb ee.k])
-
- is_expected.to match_array(%w[ee/wine.rb ee/lib/ido.rb])
- end
- end
-
describe '#file_lines' do
let(:filename) { 'spec/foo_spec.rb' }
let(:file_spy) { spy }
diff --git a/tooling/danger/changelog.rb b/tooling/danger/changelog.rb
deleted file mode 100644
index 6a392afac1384..0000000000000
--- a/tooling/danger/changelog.rb
+++ /dev/null
@@ -1,232 +0,0 @@
-# frozen_string_literal: true
-
-require 'gitlab/dangerfiles/title_linting'
-
-module Tooling
- module Danger
- module Changelog
- NO_CHANGELOG_LABELS = [
- 'type::tooling',
- 'tooling::pipelines',
- 'tooling::workflow',
- 'ci-build',
- 'meta'
- ].freeze
- NO_CHANGELOG_CATEGORIES = %i[docs none].freeze
- CHANGELOG_TRAILER_REGEX = /^(?<name>Changelog):\s*(?<category>.+)$/i.freeze
- CHANGELOG_EE_TRAILER_REGEX = /^EE: true$/.freeze
- CHANGELOG_MODIFIED_URL_TEXT = "**CHANGELOG.md was edited.** Please remove the additions and follow the [changelog guidelines](https://docs.gitlab.com/ee/development/changelog.html).\n\n"
- CHANGELOG_MISSING_URL_TEXT = "**[CHANGELOG missing](https://docs.gitlab.com/ee/development/changelog.html)**:\n\n"
-
- OPTIONAL_CHANGELOG_MESSAGE = {
- local: "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.",
- ci: <<~MSG
- If you want to create a changelog entry for GitLab FOSS, add the `Changelog` trailer to the commit message you want to add to the changelog.
-
- If you want to create a changelog entry for GitLab EE, also [add the `EE: true` trailer](https://docs.gitlab.com/ee/development/changelog.html#gitlab-enterprise-changes) to your commit message.
-
- 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.
- MSG
- }.freeze
- SEE_DOC = "See the [changelog documentation](https://docs.gitlab.com/ee/development/changelog.html)."
-
- REQUIRED_CHANGELOG_REASONS = {
- db_changes: 'introduces a database migration',
- feature_flag_removed: 'removes a feature flag'
- }.freeze
- REQUIRED_CHANGELOG_MESSAGE = {
- local: "This merge request requires a changelog entry because it [%<reason>s](https://docs.gitlab.com/ee/development/changelog.html#what-warrants-a-changelog-entry).",
- ci: <<~MSG
- To create a changelog entry, add the `Changelog` trailer to one of your Git commit messages.
-
- This merge request requires a changelog entry because it [%<reason>s](https://docs.gitlab.com/ee/development/changelog.html#what-warrants-a-changelog-entry).
- MSG
- }.freeze
-
- CATEGORIES = YAML
- .load_file(File.expand_path('../../.gitlab/changelog_config.yml', __dir__))
- .fetch('categories')
- .keys
- .freeze
-
- class ChangelogCheckResult
- attr_reader :errors, :warnings, :markdowns, :messages
-
- def initialize(errors: [], warnings: [], markdowns: [], messages: [])
- @errors = errors
- @warnings = warnings
- @markdowns = markdowns
- @messages = messages
- end
- private_class_method :new
-
- def self.empty
- new
- end
-
- def self.error(error)
- new(errors: [error])
- end
-
- def self.warning(warning)
- new(warnings: [warning])
- end
-
- def error(error)
- errors << error
- end
-
- def warning(warning)
- warnings << warning
- end
-
- def markdown(markdown)
- markdowns << markdown
- end
-
- def message(message)
- messages << message
- end
- end
-
- # rubocop:disable Style/SignalException
- def check!
- if git.modified_files.include?("CHANGELOG.md")
- fail modified_text
- end
-
- if present?
- add_danger_messages(check_changelog_path)
- elsif required?
- required_texts.each { |_, text| fail(text) } # rubocop:disable Lint/UnreachableLoop
- elsif optional?
- message optional_text
- end
-
- check_changelog_commit_categories
- end
- # rubocop:enable Style/SignalException
-
- # rubocop:disable Style/SignalException
- def add_danger_messages(check_result)
- check_result.errors.each { |error| fail(error) } # rubocop:disable Lint/UnreachableLoop
- check_result.warnings.each { |warning| warn(warning) }
- check_result.markdowns.each { |markdown_hash| markdown(**markdown_hash) }
- check_result.messages.each { |text| message(text) }
- end
- # rubocop:enable Style/SignalException
-
- def check_changelog_commit_categories
- changelog_commits.each do |commit|
- add_danger_messages(check_changelog_trailer(commit))
- end
- end
-
- def check_changelog_trailer(commit)
- trailer = commit.message.match(CHANGELOG_TRAILER_REGEX)
- name = trailer[:name]
- category = trailer[:category]
-
- unless name == 'Changelog'
- return ChangelogCheckResult.error("The changelog trailer for commit #{commit.sha} must be `Changelog` (starting with a capital C), not `#{name}`")
- end
-
- return ChangelogCheckResult.empty if CATEGORIES.include?(category)
-
- ChangelogCheckResult.error("Commit #{commit.sha} uses an invalid changelog category: #{category}")
- end
-
- def check_changelog_path
- check_result = ChangelogCheckResult.empty
- return check_result unless present?
-
- ee_changes = project_helper.all_ee_changes.dup
-
- if ee_changes.any? && !ee_changelog? && !required?
- check_result.warning("This MR changes code in `ee/`, but its Changelog commit is missing the [`EE: true` trailer](https://docs.gitlab.com/ee/development/changelog.html#gitlab-enterprise-changes). Consider adding it to your Changelog commits.")
- end
-
- if ee_changes.empty? && ee_changelog?
- check_result.warning("This MR has a Changelog commit for EE, but no code changes in `ee/`. Consider removing the `EE: true` trailer from your commits.")
- end
-
- if ee_changes.any? && ee_changelog? && required_reasons.include?(:db_changes)
- check_result.warning("This MR has a Changelog commit with the `EE: true` trailer, but there are database changes which [requires](https://docs.gitlab.com/ee/development/changelog.html#what-warrants-a-changelog-entry) the Changelog commit to not have the `EE: true` trailer. Consider removing the `EE: true` trailer from your commits.")
- end
-
- check_result
- end
-
- def required_reasons
- [].tap do |reasons|
- reasons << :db_changes if helper.changes.added.has_category?(:migration)
- reasons << :feature_flag_removed if helper.changes.deleted.has_category?(:feature_flag)
- end
- end
-
- def required?
- required_reasons.any?
- end
-
- def optional?
- categories_need_changelog? && mr_without_no_changelog_label?
- end
-
- def present?
- valid_changelog_commits.any?
- end
-
- def changelog_commits
- git.commits.select do |commit|
- commit.message.match?(CHANGELOG_TRAILER_REGEX)
- end
- end
-
- def valid_changelog_commits
- changelog_commits.select do |commit|
- trailer = commit.message.match(CHANGELOG_TRAILER_REGEX)
-
- CATEGORIES.include?(trailer[:category])
- end
- end
-
- def ee_changelog?
- changelog_commits.any? do |commit|
- commit.message.match?(CHANGELOG_EE_TRAILER_REGEX)
- end
- end
-
- def modified_text
- CHANGELOG_MODIFIED_URL_TEXT +
- (helper.ci? ? format(OPTIONAL_CHANGELOG_MESSAGE[:ci]) : OPTIONAL_CHANGELOG_MESSAGE[:local])
- end
-
- def required_texts
- required_reasons.each_with_object({}) do |required_reason, memo|
- memo[required_reason] =
- CHANGELOG_MISSING_URL_TEXT +
- (helper.ci? ? format(REQUIRED_CHANGELOG_MESSAGE[:ci], reason: REQUIRED_CHANGELOG_REASONS.fetch(required_reason)) : REQUIRED_CHANGELOG_MESSAGE[:local])
- end
- end
-
- def optional_text
- CHANGELOG_MISSING_URL_TEXT +
- (helper.ci? ? format(OPTIONAL_CHANGELOG_MESSAGE[:ci]) : OPTIONAL_CHANGELOG_MESSAGE[:local])
- end
-
- private
-
- def read_file(path)
- File.read(path)
- end
-
- def categories_need_changelog?
- (helper.changes.categories - NO_CHANGELOG_CATEGORIES).any?
- end
-
- def mr_without_no_changelog_label?
- (helper.mr_labels & NO_CHANGELOG_LABELS).empty?
- end
- end
- end
-end
diff --git a/tooling/danger/project_helper.rb b/tooling/danger/project_helper.rb
index a597ef2b0475a..01ee444b34276 100644
--- a/tooling/danger/project_helper.rb
+++ b/tooling/danger/project_helper.rb
@@ -4,7 +4,6 @@ module Tooling
module Danger
module ProjectHelper
LOCAL_RULES ||= %w[
- changelog
ci_config
database
documentation
@@ -194,18 +193,10 @@ def rule_names
helper.ci? ? LOCAL_RULES | CI_ONLY_RULES : LOCAL_RULES
end
- def all_ee_changes
- helper.changes.files.grep(%r{\Aee/})
- end
-
def file_lines(filename)
read_file(filename).lines(chomp: true)
end
- def labels_to_add
- @labels_to_add ||= []
- end
-
private
def read_file(filename)
--
GitLab