Commit 59d5f1b3 authored by Rémy Coutable's avatar Rémy Coutable

danger: Use changelog rule from gitlab-dangerfiles

Changelog: other
Signed-off-by: default avatarRémy Coutable <remy@rymai.me>
parent ab1491be
...@@ -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
......
This diff is collapsed.
...@@ -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