Move the changelog Dangerfile logic to a fully-tested helper module
This helps test the changelog rules and prevent regressions. Signed-off-by: Rémy Coutable <remy@rymai.me>
This commit is contained in:
Rémy Coutable
Albert Salim
parent
69fa381d43
commit
fefe7cc429
|
|
@ -1,135 +1,3 @@
|
|||
# frozen_string_literal: true
|
||||
# rubocop:disable Style/SignalException
|
||||
|
||||
require 'yaml'
|
||||
|
||||
SEE_DOC = "See the [changelog documentation](https://docs.gitlab.com/ee/development/changelog.html)."
|
||||
|
||||
SUGGEST_MR_COMMENT = <<~SUGGEST_COMMENT
|
||||
```suggestion
|
||||
merge_request: %<mr_iid>s
|
||||
```
|
||||
|
||||
#{SEE_DOC}
|
||||
SUGGEST_COMMENT
|
||||
|
||||
CATEGORIES = YAML
|
||||
.load_file(File.expand_path('../../.gitlab/changelog_config.yml', __dir__))
|
||||
.fetch('categories')
|
||||
.keys
|
||||
.freeze
|
||||
|
||||
def check_changelog_trailer(commit)
|
||||
trailer = commit.message.match(/^Changelog:\s*(?<category>.+)$/)
|
||||
|
||||
return :missing if trailer.nil? || trailer[:category].nil?
|
||||
|
||||
category = trailer[:category]
|
||||
|
||||
return :valid if CATEGORIES.include?(category)
|
||||
|
||||
self.fail(
|
||||
"Commit #{commit.sha} uses an invalid changelog category: #{category}"
|
||||
)
|
||||
|
||||
:invalid
|
||||
end
|
||||
|
||||
def check_changelog_yaml(path)
|
||||
raw_file = File.read(path)
|
||||
yaml = YAML.safe_load(raw_file)
|
||||
yaml_merge_request = yaml["merge_request"].to_s
|
||||
|
||||
fail "`title` should be set, in #{helper.html_link(path)}! #{SEE_DOC}" if yaml["title"].nil?
|
||||
fail "`type` should be set, in #{helper.html_link(path)}! #{SEE_DOC}" if yaml["type"].nil?
|
||||
|
||||
return if helper.security_mr?
|
||||
return if helper.mr_iid.empty?
|
||||
|
||||
cherry_pick_against_stable_branch = helper.cherry_pick_mr? && helper.stable_branch?
|
||||
|
||||
if yaml_merge_request.empty?
|
||||
mr_line = raw_file.lines.find_index("merge_request:\n")
|
||||
|
||||
if mr_line
|
||||
markdown(format(SUGGEST_MR_COMMENT, mr_iid: helper.mr_iid), file: path, line: mr_line.succ)
|
||||
else
|
||||
message "Consider setting `merge_request` to #{helper.mr_iid} in #{helper.html_link(path)}. #{SEE_DOC}"
|
||||
end
|
||||
elsif yaml_merge_request != helper.mr_iid && !cherry_pick_against_stable_branch
|
||||
fail "Merge request ID was not set to #{helper.mr_iid}! #{SEE_DOC}"
|
||||
end
|
||||
rescue Psych::Exception
|
||||
# YAML could not be parsed, fail the build.
|
||||
fail "#{helper.html_link(path)} isn't valid YAML! #{SEE_DOC}"
|
||||
rescue StandardError => e
|
||||
warn "There was a problem trying to check the Changelog. Exception: #{e.class.name} - #{e.message}"
|
||||
end
|
||||
|
||||
def check_changelog_path(path)
|
||||
ee_changes = project_helper.all_ee_changes.dup
|
||||
ee_changes.delete(path)
|
||||
|
||||
if ee_changes.any? && !changelog.ee_changelog? && !changelog.required?
|
||||
warn "This MR has a Changelog file outside `ee/`, but code changes in `ee/`. Consider moving the Changelog file into `ee/`."
|
||||
end
|
||||
|
||||
if ee_changes.empty? && changelog.ee_changelog?
|
||||
warn "This MR has a Changelog file in `ee/`, but no code changes in `ee/`. Consider moving the Changelog file outside `ee/`."
|
||||
end
|
||||
|
||||
if ee_changes.any? && changelog.ee_changelog? && changelog.required_reasons.include?(:db_changes)
|
||||
warn "This MR has a Changelog file inside `ee/`, but there are database changes which [requires](https://docs.gitlab.com/ee/development/changelog.html#what-warrants-a-changelog-entry) the Changelog placement to be outside of `ee/`. Consider moving the Changelog file outside `ee/`."
|
||||
end
|
||||
end
|
||||
|
||||
if git.modified_files.include?("CHANGELOG.md")
|
||||
fail changelog.modified_text
|
||||
end
|
||||
|
||||
changelog_found = changelog.found
|
||||
|
||||
if changelog_found
|
||||
check_changelog_yaml(changelog_found)
|
||||
check_changelog_path(changelog_found)
|
||||
elsif changelog.required?
|
||||
changelog.required_texts.each { |_, text| fail(text) } # rubocop:disable Lint/UnreachableLoop
|
||||
elsif changelog.optional?
|
||||
message changelog.optional_text
|
||||
end
|
||||
|
||||
if helper.ci? && (changelog.required? || changelog.optional?)
|
||||
checked = 0
|
||||
|
||||
git.commits.each do |commit|
|
||||
case check_changelog_trailer(commit)
|
||||
when :valid, :invalid
|
||||
checked += 1
|
||||
end
|
||||
end
|
||||
|
||||
if checked == 0
|
||||
message <<~MSG
|
||||
We are in the process of rolling out a new workflow for adding changelog entries. This new workflow uses Git commit subjects and Git trailers to generate changelogs. This new approach will soon replace the current YAML based approach.
|
||||
|
||||
To ease the transition process, we recommend you start using both the old and new approach in parallel. This is not required at this time, but will make it easier to transition to the new approach in the future. To do so, pick the commit that should go in the changelog and add a `Changelog` trailer to it. For example:
|
||||
|
||||
```
|
||||
This is my commit's subject line
|
||||
|
||||
This is the optional commit body.
|
||||
|
||||
Changelog: added
|
||||
```
|
||||
|
||||
The value of the `Changelog` trailer should be one of the following: added, fixed, changed, deprecated, removed, security, performance, other.
|
||||
|
||||
For more information, take a look at the following resources:
|
||||
|
||||
- `https://gitlab.com/gitlab-com/gl-infra/delivery/-/issues/1564`
|
||||
- https://docs.gitlab.com/ee/api/repositories.html#generate-changelog-data
|
||||
|
||||
If you'd like to see the new approach in action, take a look at the commits in [the Omnibus repository](https://gitlab.com/gitlab-org/omnibus-gitlab/-/commits/master).
|
||||
MSG
|
||||
end
|
||||
end
|
||||
changelog.check!
|
||||
|
|
|
|||
|
|
@ -18,6 +18,204 @@
|
|||
allow(changelog).to receive(:project_helper).and_return(fake_project_helper)
|
||||
end
|
||||
|
||||
describe '#check_changelog_trailer' do
|
||||
subject { changelog.check_changelog_trailer(commit) }
|
||||
|
||||
context "when commit doesn't include a changelog trailer" do
|
||||
let(:commit) { double('commit', message: "Hello world") }
|
||||
|
||||
it { is_expected.to be_nil }
|
||||
end
|
||||
|
||||
context "when commit include a changelog trailer with no category" do
|
||||
let(:commit) { double('commit', message: "Hello world\n\nChangelog:") }
|
||||
|
||||
it { is_expected.to be_nil }
|
||||
end
|
||||
|
||||
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
|
||||
|
||||
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_yaml' do
|
||||
let(:changelog_path) { 'ee/changelogs/unreleased/entry.yml' }
|
||||
let(:changes) { changes_class.new([change_class.new(changelog_path, :added, :changelog)]) }
|
||||
let(:yaml_title) { 'Fix changelog Dangerfile to convert MR IID to a string before comparison' }
|
||||
let(:yaml_merge_request) { 60899 }
|
||||
let(:mr_iid) { '60899' }
|
||||
let(:yaml_type) { 'fixed' }
|
||||
let(:yaml) do
|
||||
<<~YAML
|
||||
---
|
||||
title: #{yaml_title}
|
||||
merge_request: #{yaml_merge_request}
|
||||
author:
|
||||
type: #{yaml_type}
|
||||
YAML
|
||||
end
|
||||
|
||||
before do
|
||||
allow(changelog).to receive(:present?).and_return(true)
|
||||
allow(changelog).to receive(:changelog_path).and_return(changelog_path)
|
||||
allow(changelog).to receive(:read_file).with(changelog_path).and_return(yaml)
|
||||
allow(fake_helper).to receive(:security_mr?).and_return(false)
|
||||
allow(fake_helper).to receive(:mr_iid).and_return(mr_iid)
|
||||
allow(fake_helper).to receive(:cherry_pick_mr?).and_return(false)
|
||||
allow(fake_helper).to receive(:stable_branch?).and_return(false)
|
||||
allow(fake_helper).to receive(:html_link).with(changelog_path).and_return(changelog_path)
|
||||
end
|
||||
|
||||
subject { changelog.check_changelog_yaml }
|
||||
|
||||
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 "when YAML is invalid" do
|
||||
let(:yaml) { '{ foo bar]' }
|
||||
|
||||
it { is_expected.to have_attributes(errors: ["#{changelog_path} isn't valid YAML! #{described_class::SEE_DOC}"]) }
|
||||
end
|
||||
|
||||
context "when a StandardError is raised" do
|
||||
before do
|
||||
allow(changelog).to receive(:read_file).and_raise(StandardError, "Fail!")
|
||||
end
|
||||
|
||||
it { is_expected.to have_attributes(warnings: ["There was a problem trying to check the Changelog. Exception: StandardError - Fail!"]) }
|
||||
end
|
||||
|
||||
context "when YAML title is nil" do
|
||||
let(:yaml_title) { '' }
|
||||
|
||||
it { is_expected.to have_attributes(errors: ["`title` should be set, in #{changelog_path}! #{described_class::SEE_DOC}"]) }
|
||||
end
|
||||
|
||||
context "when YAML type is nil" do
|
||||
let(:yaml_type) { '' }
|
||||
|
||||
it { is_expected.to have_attributes(errors: ["`type` should be set, in #{changelog_path}! #{described_class::SEE_DOC}"]) }
|
||||
end
|
||||
|
||||
context "when on a security MR" do
|
||||
let(:yaml_merge_request) { '' }
|
||||
|
||||
before do
|
||||
allow(fake_helper).to receive(:security_mr?).and_return(true)
|
||||
end
|
||||
|
||||
it { is_expected.to have_attributes(errors: [], warnings: [], markdowns: [], messages: []) }
|
||||
end
|
||||
|
||||
context "when MR IID is empty" do
|
||||
before do
|
||||
allow(fake_helper).to receive(:mr_iid).and_return("")
|
||||
end
|
||||
|
||||
it { is_expected.to have_attributes(errors: [], warnings: [], markdowns: [], messages: []) }
|
||||
end
|
||||
|
||||
context "when YAML MR IID is empty" do
|
||||
let(:yaml_merge_request) { '' }
|
||||
|
||||
context "and YAML includes a merge_request: line" do
|
||||
it { is_expected.to have_attributes(markdowns: [{ msg: format(described_class::SUGGEST_MR_COMMENT, mr_iid: fake_helper.mr_iid), file: changelog_path, line: 3 }]) }
|
||||
end
|
||||
|
||||
context "and YAML does not include a merge_request: line" do
|
||||
let(:yaml) do
|
||||
<<~YAML
|
||||
---
|
||||
title: #{yaml_title}
|
||||
author:
|
||||
type: #{yaml_type}
|
||||
YAML
|
||||
end
|
||||
|
||||
it { is_expected.to have_attributes(messages: ["Consider setting `merge_request` to #{mr_iid} in #{changelog_path}. #{described_class::SEE_DOC}"]) }
|
||||
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
|
||||
let(:changelog_change) { change_class.new('changelogs/unreleased/entry.yml', :added, :changelog) }
|
||||
|
||||
before do
|
||||
allow(changelog).to receive(:required?).and_return(false)
|
||||
end
|
||||
|
||||
it { is_expected.to have_attributes(warnings: ["This MR has a Changelog file outside `ee/`, but code changes in `ee/`. Consider moving the Changelog file into `ee/`."]) }
|
||||
end
|
||||
|
||||
context "and a EE changelog" do
|
||||
let(:changelog_change) { change_class.new('ee/changelogs/unreleased/entry.yml', :added, :changelog) }
|
||||
|
||||
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 file inside `ee/`, but there are database changes which [requires](https://docs.gitlab.com/ee/development/changelog.html#what-warrants-a-changelog-entry) the Changelog placement to be outside of `ee/`. Consider moving the Changelog file outside `ee/`."]) }
|
||||
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
|
||||
let(:changelog_change) { change_class.new('changelogs/unreleased/entry.yml', :added, :changelog) }
|
||||
|
||||
it { is_expected.to have_attributes(errors: [], warnings: [], markdowns: [], messages: []) }
|
||||
end
|
||||
|
||||
context "and a EE changelog" do
|
||||
let(:changelog_change) { change_class.new('ee/changelogs/unreleased/entry.yml', :added, :changelog) }
|
||||
|
||||
it { is_expected.to have_attributes(warnings: ["This MR has a Changelog file in `ee/`, but no code changes in `ee/`. Consider moving the Changelog file outside `ee/`."]) }
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
describe '#required_reasons' do
|
||||
subject { changelog.required_reasons }
|
||||
|
||||
|
|
@ -126,8 +324,8 @@
|
|||
end
|
||||
end
|
||||
|
||||
describe '#found' do
|
||||
subject { changelog.found }
|
||||
describe '#present?' do
|
||||
subject { changelog.present? }
|
||||
|
||||
context 'added files contain a changelog' do
|
||||
let(:changes) { changes_class.new([change_class.new('foo', :added, :changelog)]) }
|
||||
|
|
@ -138,7 +336,7 @@
|
|||
context 'added files do not contain a changelog' do
|
||||
let(:changes) { changes_class.new([change_class.new('foo', :added, :backend)]) }
|
||||
|
||||
it { is_expected.to eq(nil) }
|
||||
it { is_expected.to be_falsy }
|
||||
end
|
||||
end
|
||||
|
||||
|
|
@ -158,6 +356,22 @@
|
|||
end
|
||||
end
|
||||
|
||||
describe '#changelog_path' do
|
||||
subject { changelog.changelog_path }
|
||||
|
||||
context 'added files contain a changelog' do
|
||||
let(:changes) { changes_class.new([change_class.new('foo', :added, :changelog)]) }
|
||||
|
||||
it { is_expected.to eq('foo') }
|
||||
end
|
||||
|
||||
context 'added files do not contain a changelog' do
|
||||
let(:changes) { changes_class.new([change_class.new('foo', :added, :backend)]) }
|
||||
|
||||
it { is_expected.to be_nil }
|
||||
end
|
||||
end
|
||||
|
||||
describe '#modified_text' do
|
||||
subject { changelog.modified_text }
|
||||
|
||||
|
|
|
|||
|
|
@ -256,7 +256,9 @@
|
|||
subject { project_helper.all_ee_changes }
|
||||
|
||||
it 'returns all changed files starting with ee/' do
|
||||
expect(fake_helper).to receive(:all_changed_files).and_return(%w[fr/ee/beer.rb ee/wine.rb ee/lib/ido.rb ee.k])
|
||||
changes = double
|
||||
expect(project_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
|
||||
|
|
|
|||
|
|
@ -13,9 +13,10 @@ module Changelog
|
|||
'meta'
|
||||
].freeze
|
||||
NO_CHANGELOG_CATEGORIES = %i[docs none].freeze
|
||||
CHANGELOG_TRAILER_REGEX = /^Changelog:\s*(?<category>.+)$/.freeze
|
||||
CREATE_CHANGELOG_COMMAND = 'bin/changelog -m %<mr_iid>s "%<mr_title>s"'
|
||||
CREATE_EE_CHANGELOG_COMMAND = 'bin/changelog --ee -m %<mr_iid>s "%<mr_title>s"'
|
||||
CHANGELOG_MODIFIED_URL_TEXT = "**CHANGELOG.md was edited.** Please remove the additions and create a CHANGELOG entry.\n\n"
|
||||
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 = {
|
||||
|
|
@ -32,6 +33,36 @@ module Changelog
|
|||
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
|
||||
CHANGELOG_NEW_WORKFLOW_MESSAGE = <<~MSG
|
||||
We are in the process of rolling out a new workflow for adding changelog entries. This new workflow uses Git commit subjects and Git trailers to generate changelogs. This new approach will soon replace the current YAML based approach.
|
||||
|
||||
To ease the transition process, we recommend you start using both the old and new approach in parallel. This is not required at this time, but will make it easier to transition to the new approach in the future. To do so, pick the commit that should go in the changelog and add a `Changelog` trailer to it. For example:
|
||||
|
||||
```
|
||||
This is my commit's subject line
|
||||
|
||||
This is the optional commit body.
|
||||
|
||||
Changelog: added
|
||||
```
|
||||
|
||||
The value of the `Changelog` trailer should be one of the following: added, fixed, changed, deprecated, removed, security, performance, other.
|
||||
|
||||
For more information, take a look at the following resources:
|
||||
|
||||
- `https://gitlab.com/gitlab-com/gl-infra/delivery/-/issues/1564`
|
||||
- https://docs.gitlab.com/ee/api/repositories.html#generate-changelog-data
|
||||
|
||||
If you'd like to see the new approach in action, take a look at the commits in [the Omnibus repository](https://gitlab.com/gitlab-org/omnibus-gitlab/-/commits/master).
|
||||
MSG
|
||||
SEE_DOC = "See the [changelog documentation](https://docs.gitlab.com/ee/development/changelog.html)."
|
||||
SUGGEST_MR_COMMENT = <<~SUGGEST_COMMENT
|
||||
```suggestion
|
||||
merge_request: %<mr_iid>s
|
||||
```
|
||||
|
||||
#{SEE_DOC}
|
||||
SUGGEST_COMMENT
|
||||
|
||||
REQUIRED_CHANGELOG_REASONS = {
|
||||
db_changes: 'introduces a database migration',
|
||||
|
|
@ -48,6 +79,169 @@ module Changelog
|
|||
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_yaml)
|
||||
add_danger_messages(check_changelog_path)
|
||||
elsif required?
|
||||
required_texts.each { |_, text| fail(text) } # rubocop:disable Lint/UnreachableLoop
|
||||
elsif optional?
|
||||
message optional_text
|
||||
end
|
||||
|
||||
return unless helper.ci?
|
||||
|
||||
if required? || optional?
|
||||
checked = 0
|
||||
|
||||
git.commits.each do |commit|
|
||||
check_result = check_changelog_trailer(commit)
|
||||
next if check_result.nil?
|
||||
|
||||
checked += 1
|
||||
add_danger_messages(check_result)
|
||||
end
|
||||
|
||||
if checked == 0
|
||||
message CHANGELOG_NEW_WORKFLOW_MESSAGE
|
||||
end
|
||||
end
|
||||
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_trailer(commit)
|
||||
trailer = commit.message.match(CHANGELOG_TRAILER_REGEX)
|
||||
|
||||
return if trailer.nil? || trailer[:category].nil?
|
||||
|
||||
category = trailer[:category]
|
||||
|
||||
return ChangelogCheckResult.empty if CATEGORIES.include?(category)
|
||||
|
||||
ChangelogCheckResult.error("Commit #{commit.sha} uses an invalid changelog category: #{category}")
|
||||
end
|
||||
|
||||
def check_changelog_yaml
|
||||
check_result = ChangelogCheckResult.empty
|
||||
return check_result unless present?
|
||||
|
||||
raw_file = read_file(changelog_path)
|
||||
yaml = YAML.safe_load(raw_file)
|
||||
yaml_merge_request = yaml["merge_request"].to_s
|
||||
|
||||
check_result.error("`title` should be set, in #{helper.html_link(changelog_path)}! #{SEE_DOC}") if yaml["title"].nil?
|
||||
check_result.error("`type` should be set, in #{helper.html_link(changelog_path)}! #{SEE_DOC}") if yaml["type"].nil?
|
||||
|
||||
return check_result if helper.security_mr? || helper.mr_iid.empty?
|
||||
|
||||
check_changelog_yaml_merge_request(raw_file: raw_file, yaml_merge_request: yaml_merge_request, check_result: check_result)
|
||||
rescue Psych::Exception
|
||||
# YAML could not be parsed, fail the build.
|
||||
ChangelogCheckResult.error("#{helper.html_link(changelog_path)} isn't valid YAML! #{SEE_DOC}")
|
||||
rescue StandardError => e
|
||||
ChangelogCheckResult.warning("There was a problem trying to check the Changelog. Exception: #{e.class.name} - #{e.message}")
|
||||
end
|
||||
|
||||
def check_changelog_yaml_merge_request(raw_file:, yaml_merge_request:, check_result:)
|
||||
cherry_pick_against_stable_branch = helper.cherry_pick_mr? && helper.stable_branch?
|
||||
|
||||
if yaml_merge_request.empty?
|
||||
mr_line = raw_file.lines.find_index { |line| line =~ /merge_request:\s*\n/ }
|
||||
|
||||
if mr_line
|
||||
check_result.markdown(msg: format(SUGGEST_MR_COMMENT, mr_iid: helper.mr_iid), file: changelog_path, line: mr_line.succ)
|
||||
else
|
||||
check_result.message("Consider setting `merge_request` to #{helper.mr_iid} in #{helper.html_link(changelog_path)}. #{SEE_DOC}")
|
||||
end
|
||||
elsif yaml_merge_request != helper.mr_iid && !cherry_pick_against_stable_branch
|
||||
check_result.error("Merge request ID was not set to #{helper.mr_iid}! #{SEE_DOC}")
|
||||
end
|
||||
|
||||
check_result
|
||||
end
|
||||
|
||||
def check_changelog_path
|
||||
check_result = ChangelogCheckResult.empty
|
||||
return check_result unless present?
|
||||
|
||||
ee_changes = project_helper.all_ee_changes.dup
|
||||
ee_changes.delete(changelog_path)
|
||||
|
||||
if ee_changes.any? && !ee_changelog? && !required?
|
||||
check_result.warning("This MR has a Changelog file outside `ee/`, but code changes in `ee/`. Consider moving the Changelog file into `ee/`.")
|
||||
end
|
||||
|
||||
if ee_changes.empty? && ee_changelog?
|
||||
check_result.warning("This MR has a Changelog file in `ee/`, but no code changes in `ee/`. Consider moving the Changelog file outside `ee/`.")
|
||||
end
|
||||
|
||||
if ee_changes.any? && ee_changelog? && required_reasons.include?(:db_changes)
|
||||
check_result.warning("This MR has a Changelog file inside `ee/`, but there are database changes which [requires](https://docs.gitlab.com/ee/development/changelog.html#what-warrants-a-changelog-entry) the Changelog placement to be outside of `ee/`. Consider moving the Changelog file outside `ee/`.")
|
||||
end
|
||||
|
||||
check_result
|
||||
end
|
||||
|
||||
def required_reasons
|
||||
[].tap do |reasons|
|
||||
reasons << :db_changes if project_helper.changes.added.has_category?(:migration)
|
||||
|
|
@ -60,15 +254,19 @@ def required?
|
|||
end
|
||||
|
||||
def optional?
|
||||
categories_need_changelog? && without_no_changelog_label?
|
||||
categories_need_changelog? && mr_without_no_changelog_label?
|
||||
end
|
||||
|
||||
def found
|
||||
@found ||= project_helper.changes.added.by_category(:changelog).files.first
|
||||
def present?
|
||||
!!changelog_path
|
||||
end
|
||||
|
||||
def ee_changelog?
|
||||
found.start_with?('ee/')
|
||||
changelog_path.start_with?('ee/')
|
||||
end
|
||||
|
||||
def changelog_path
|
||||
@changelog_path ||= project_helper.changes.added.by_category(:changelog).files.first
|
||||
end
|
||||
|
||||
def modified_text
|
||||
|
|
@ -91,6 +289,10 @@ def optional_text
|
|||
|
||||
private
|
||||
|
||||
def read_file(path)
|
||||
File.read(path)
|
||||
end
|
||||
|
||||
def sanitized_mr_title
|
||||
Gitlab::Dangerfiles::TitleLinting.sanitize_mr_title(helper.mr_title)
|
||||
end
|
||||
|
|
@ -99,7 +301,7 @@ def categories_need_changelog?
|
|||
(project_helper.changes.categories - NO_CHANGELOG_CATEGORIES).any?
|
||||
end
|
||||
|
||||
def without_no_changelog_label?
|
||||
def mr_without_no_changelog_label?
|
||||
(helper.mr_labels & NO_CHANGELOG_LABELS).empty?
|
||||
end
|
||||
end
|
||||
|
|
|
|||
|
|
@ -168,7 +168,7 @@ def rule_names
|
|||
end
|
||||
|
||||
def all_ee_changes
|
||||
helper.all_changed_files.grep(%r{\Aee/})
|
||||
changes.files.grep(%r{\Aee/})
|
||||
end
|
||||
|
||||
def project_name
|
||||
|
|
|
|||
Loading…
Reference in a new issue