Commit 778297d8 authored by Rémy Coutable's avatar Rémy Coutable

Merge branch 'use-changelog-rule-from-gitlab-dangerfiles' into 'master'

danger: Use changelog rule from gitlab-dangerfiles

See merge request gitlab-org/gitlab!81290
parents 62c7a73b 59d5f1b3
...@@ -24,24 +24,3 @@ return if helper.release_automation? ...@@ -24,24 +24,3 @@ return if helper.release_automation?
project_helper.rule_names.each do |rule| project_helper.rule_names.each do |rule|
danger.import_dangerfile(path: File.join('danger', rule)) danger.import_dangerfile(path: File.join('danger', rule))
end 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
...@@ -398,7 +398,7 @@ group :development, :test do ...@@ -398,7 +398,7 @@ group :development, :test do
end end
group :development, :test, :danger do group :development, :test, :danger do
gem 'gitlab-dangerfiles', '~> 2.8.0', require: false gem 'gitlab-dangerfiles', '~> 2.9.3', require: false
end end
group :development, :test, :coverage do group :development, :test, :coverage do
......
...@@ -221,7 +221,7 @@ GEM ...@@ -221,7 +221,7 @@ GEM
css_parser (1.7.0) css_parser (1.7.0)
addressable addressable
daemons (1.3.1) daemons (1.3.1)
danger (8.4.2) danger (8.4.3)
claide (~> 1.0) claide (~> 1.0)
claide-plugins (>= 0.9.2) claide-plugins (>= 0.9.2)
colored2 (~> 3.1) colored2 (~> 3.1)
...@@ -460,7 +460,7 @@ GEM ...@@ -460,7 +460,7 @@ GEM
terminal-table (~> 1.5, >= 1.5.1) terminal-table (~> 1.5, >= 1.5.1)
gitlab-chronic (0.10.5) gitlab-chronic (0.10.5)
numerizer (~> 0.2) numerizer (~> 0.2)
gitlab-dangerfiles (2.8.0) gitlab-dangerfiles (2.9.3)
danger (>= 8.3.1) danger (>= 8.3.1)
danger-gitlab (>= 8.0.0) danger-gitlab (>= 8.0.0)
gitlab-experiment (0.7.0) gitlab-experiment (0.7.0)
...@@ -1470,7 +1470,7 @@ DEPENDENCIES ...@@ -1470,7 +1470,7 @@ DEPENDENCIES
gitaly (~> 14.8.0.pre.rc1) gitaly (~> 14.8.0.pre.rc1)
github-markup (~> 1.7.0) github-markup (~> 1.7.0)
gitlab-chronic (~> 0.10.5) gitlab-chronic (~> 0.10.5)
gitlab-dangerfiles (~> 2.8.0) gitlab-dangerfiles (~> 2.9.3)
gitlab-experiment (~> 0.7.0) gitlab-experiment (~> 0.7.0)
gitlab-fog-azure-rm (~> 1.2.0) gitlab-fog-azure-rm (~> 1.2.0)
gitlab-labkit (~> 0.22.0) gitlab-labkit (~> 0.22.0)
......
# frozen_string_literal: true
changelog.check!
...@@ -65,7 +65,7 @@ if gitlab.mr_labels.include?('database') || db_paths_to_review.any? ...@@ -65,7 +65,7 @@ if gitlab.mr_labels.include?('database') || db_paths_to_review.any?
markdown(DB_REMOVE_MESSAGE) markdown(DB_REMOVE_MESSAGE)
end end
unless helper.has_database_scoped_labels? unless helper.has_scoped_label_with_scope?("database")
project_helper.labels_to_add << 'database::review pending' helper.labels_to_add << 'database::review pending'
end end
end end
...@@ -58,7 +58,7 @@ def message_for_feature_flag_with_group!(feature_flag:, mr_group_label:) ...@@ -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) return if feature_flag.group_match_mr_label?(mr_group_label)
if mr_group_label.nil? if mr_group_label.nil?
project_helper.labels_to_add << feature_flag.group helper.labels_to_add << feature_flag.group
else 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!) 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 end
......
# 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
...@@ -19,4 +19,4 @@ return if product_intelligence_paths_to_review.empty? || product_intelligence.sk ...@@ -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? 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?
...@@ -26,4 +26,4 @@ labels_to_add = helper.changes_by_category.each_with_object([]) do |(category, _ ...@@ -26,4 +26,4 @@ labels_to_add = helper.changes_by_category.each_with_object([]) do |(category, _
memo << label memo << label
end 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?
...@@ -4,24 +4,10 @@ ...@@ -4,24 +4,10 @@
DEFAULT_BRANCH = 'master' 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 if gitlab.mr_body.size < 5
fail "Please provide a proper merge request description." fail "Please provide a proper merge request description."
end 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"] 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." 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 end
......
...@@ -121,12 +121,13 @@ to revert the change before merging! ...@@ -121,12 +121,13 @@ to revert the change before merging!
#### Adding labels via Danger #### Adding labels via Danger
NOTE: NOTE:
This is currently applicable to the [`gitlab-org/gitlab`](https://gitlab.com/gitlab-org/gitlab) This is applicable to all the projects that use the [`gitlab-dangerfiles` gem](https://rubygems.org/gems/gitlab-dangerfiles).
project only.
Danger is often used to improve MR hygiene by adding labels. Instead of calling the 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. API directly in your `Dangerfile`, add the labels to `helper.labels_to_add` set (with `helper.labels_to_add << label`
The main `Dangerfile` will then take care of adding the labels to the MR with a single API call. 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 #### Shared rules and plugins
...@@ -135,11 +136,30 @@ upstreaming them to the [`gitlab-dangerfiles`](https://gitlab.com/gitlab-org/rub ...@@ -135,11 +136,30 @@ upstreaming them to the [`gitlab-dangerfiles`](https://gitlab.com/gitlab-org/rub
#### Enable Danger on a project #### Enable Danger on a project
To enable the Dangerfile on another existing GitLab project, run the following To enable the Dangerfile on another existing GitLab project, complete the following steps:
extra steps:
1. Create a [Project access tokens](../user/project/settings/project_access_tokens.md). 1. Add [`gitlab-dangerfiles`](https://rubygems.org/gems/gitlab-dangerfiles) to your `Gemfile`.
1. Add the token as a CI/CD project variable named `DANGER_GITLAB_API_TOKEN`. 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 You should add the ~"Danger bot" label to the merge request before sending it
for review. for review.
......
...@@ -2,7 +2,7 @@ pre-push: ...@@ -2,7 +2,7 @@ pre-push:
parallel: true parallel: true
commands: commands:
danger: danger:
run: bundle exec danger dry_run run: CI_PROJECT_DIR=. bundle exec danger dry_run
eslint: eslint:
tags: frontend style tags: frontend style
files: git diff --name-only --diff-filter=d $(git merge-base origin/master HEAD)..HEAD files: git diff --name-only --diff-filter=d $(git merge-base origin/master HEAD)..HEAD
......
# 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
...@@ -274,7 +274,7 @@ RSpec.describe Tooling::Danger::ProjectHelper do ...@@ -274,7 +274,7 @@ RSpec.describe Tooling::Danger::ProjectHelper do
describe '.local_warning_message' do describe '.local_warning_message' do
it 'returns an informational message with rules that can run' 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
end end
...@@ -306,18 +306,6 @@ RSpec.describe Tooling::Danger::ProjectHelper do ...@@ -306,18 +306,6 @@ RSpec.describe Tooling::Danger::ProjectHelper do
end end
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 describe '#file_lines' do
let(:filename) { 'spec/foo_spec.rb' } let(:filename) { 'spec/foo_spec.rb' }
let(:file_spy) { spy } let(:file_spy) { spy }
......
# 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
...@@ -4,7 +4,6 @@ module Tooling ...@@ -4,7 +4,6 @@ module Tooling
module Danger module Danger
module ProjectHelper module ProjectHelper
LOCAL_RULES ||= %w[ LOCAL_RULES ||= %w[
changelog
ci_config ci_config
database database
documentation documentation
...@@ -194,18 +193,10 @@ module Tooling ...@@ -194,18 +193,10 @@ module Tooling
helper.ci? ? LOCAL_RULES | CI_ONLY_RULES : LOCAL_RULES helper.ci? ? LOCAL_RULES | CI_ONLY_RULES : LOCAL_RULES
end end
def all_ee_changes
helper.changes.files.grep(%r{\Aee/})
end
def file_lines(filename) def file_lines(filename)
read_file(filename).lines(chomp: true) read_file(filename).lines(chomp: true)
end end
def labels_to_add
@labels_to_add ||= []
end
private private
def read_file(filename) def read_file(filename)
......
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