Allow the changelog Danger rule to work locally

This will enhance the local experience by pushing the quality left.
Signed-off-by: Rémy Coutable <remy@rymai.me>

danger/changelog/Dangerfile

@@ -21,6 +21,7 @@ def check_changelog_yaml(path)
   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?
 
@@ -28,12 +29,12 @@ def check_changelog_yaml(path)
     mr_line = raw_file.lines.find_index("merge_request:\n")
 
     if mr_line
-      markdown(format(SUGGEST_MR_COMMENT, mr_iid: gitlab.mr_json["iid"]), file: path, line: mr_line.succ)
+      markdown(format(SUGGEST_MR_COMMENT, mr_iid: helper.mr_iid), file: path, line: mr_line.succ)
     else
-      message "Consider setting `merge_request` to #{gitlab.mr_json["iid"]} in #{helper.html_link(path)}. #{SEE_DOC}"
+      message "Consider setting `merge_request` to #{helper.mr_iid} in #{helper.html_link(path)}. #{SEE_DOC}"
     end
-  elsif yaml["merge_request"] != gitlab.mr_json["iid"] && !cherry_pick_against_stable_branch
-    fail "Merge request ID was not set to #{gitlab.mr_json["iid"]}! #{SEE_DOC}"
+  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.

spec/tooling/danger/changelog_spec.rb

@@ -161,23 +161,42 @@ RSpec.describe Tooling::Danger::Changelog do
   describe '#modified_text' do
     subject { changelog.modified_text }
 
-    context "when title is not changed from sanitization", :aggregate_failures do
-      let(:mr_title) { 'Fake Title' }
+    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('bin/changelog -m 1234 "Fake Title"')
+          expect(subject).to include('bin/changelog --ee -m 1234 "Fake Title"')
+        end
+      end
 
-      specify do
-        expect(subject).to include('CHANGELOG.md was edited')
-        expect(subject).to include('bin/changelog -m 1234 "Fake Title"')
-        expect(subject).to include('bin/changelog --ee -m 1234 "Fake Title"')
+      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 title needs sanitization", :aggregate_failures do
-      let(:mr_title) { 'DRAFT: Fake Title' }
+    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).to include('bin/changelog -m 1234 "Fake Title"')
-        expect(subject).to include('bin/changelog --ee -m 1234 "Fake Title"')
+        expect(subject).not_to include('bin/changelog')
       end
     end
   end
@@ -187,56 +206,116 @@ RSpec.describe Tooling::Danger::Changelog do
 
     subject { changelog.required_texts }
 
-    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('bin/changelog -m 1234 "Fake Title"')
-        expect(subject[key]).not_to include('--ee')
+    context 'when in CI context' do
+      before do
+        allow(fake_helper).to receive(:ci?).and_return(true)
       end
-    end
 
-    context 'with a new migration file' do
-      let(:changes) { changes_class.new([change_class.new('foo', :added, :migration)]) }
+      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('bin/changelog -m 1234 "Fake Title"')
+          expect(subject[key]).not_to include('--ee')
+        end
+      end
 
-      context "when title is not changed from sanitization", :aggregate_failures do
-        it_behaves_like 'changelog required text', :db_changes
+      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 "when title needs sanitization", :aggregate_failures do
-        let(:mr_title) { 'DRAFT: Fake Title' }
+      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', :db_changes
+        it_behaves_like 'changelog required text', :feature_flag_removed
       end
     end
 
-    context 'with a removed feature flag file' do
-      let(:changes) { changes_class.new([change_class.new('foo', :deleted, :feature_flag)]) }
+    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('bin/changelog')
+          expect(subject[key]).not_to include('--ee')
+        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', :feature_flag_removed
+          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 title is not changed from sanitization", :aggregate_failures do
-      let(:mr_title) { 'Fake Title' }
+    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('bin/changelog -m 1234 "Fake Title"')
+          expect(subject).to include('bin/changelog --ee -m 1234 "Fake Title"')
+        end
+      end
 
-      specify do
-        expect(subject).to include('CHANGELOG missing')
-        expect(subject).to include('bin/changelog -m 1234 "Fake Title"')
-        expect(subject).to include('bin/changelog --ee -m 1234 "Fake Title"')
+      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 title needs sanitization", :aggregate_failures do
-      let(:mr_title) { 'DRAFT: Fake Title' }
+    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')
-        expect(subject).to include('bin/changelog -m 1234 "Fake Title"')
-        expect(subject).to include('bin/changelog --ee -m 1234 "Fake Title"')
+        expect(subject).not_to include('bin/changelog')
       end
     end
   end

spec/tooling/danger/project_helper_spec.rb

@@ -203,7 +203,7 @@ RSpec.describe Tooling::Danger::ProjectHelper do
 
   describe '.local_warning_message' do
     it 'returns an informational message with rules that can run' do
-      expect(described_class.local_warning_message).to eq('==> Only the following Danger rules can be run locally: changes_size, commit_messages, database, documentation, duplicate_yarn_dependencies, eslint, karma, pajamas, pipeline, prettier, product_intelligence, utility_css')
+      expect(described_class.local_warning_message).to eq('==> Only the following Danger rules can be run locally: changelog, changes_size, commit_messages, database, documentation, duplicate_yarn_dependencies, eslint, karma, pajamas, pipeline, prettier, product_intelligence, utility_css')
     end
   end
 

tooling/danger/changelog.rb

@@ -18,29 +18,35 @@ module Tooling
       CHANGELOG_MODIFIED_URL_TEXT = "**CHANGELOG.md was edited.** Please remove the additions and create a CHANGELOG entry.\n\n"
       CHANGELOG_MISSING_URL_TEXT = "**[CHANGELOG missing](https://docs.gitlab.com/ee/development/changelog.html)**:\n\n"
 
-      OPTIONAL_CHANGELOG_MESSAGE = <<~MSG
-      If you want to create a changelog entry for GitLab FOSS, run the following:
+      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, run the following:
 
-          #{CREATE_CHANGELOG_COMMAND}
+            #{CREATE_CHANGELOG_COMMAND}
 
-      If you want to create a changelog entry for GitLab EE, run the following instead:
+        If you want to create a changelog entry for GitLab EE, run the following instead:
 
-          #{CREATE_EE_CHANGELOG_COMMAND}
+            #{CREATE_EE_CHANGELOG_COMMAND}
 
-      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
+        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
 
       REQUIRED_CHANGELOG_REASONS = {
         db_changes: 'introduces a database migration',
         feature_flag_removed: 'removes a feature flag'
       }.freeze
-      REQUIRED_CHANGELOG_MESSAGE = <<~MSG
-      To create a changelog entry, run the following:
+      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, run the following:
 
-          #{CREATE_CHANGELOG_COMMAND}
+            #{CREATE_CHANGELOG_COMMAND}
 
-      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
+        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
 
       def required_reasons
         [].tap do |reasons|
@@ -67,20 +73,20 @@ module Tooling
 
       def modified_text
         CHANGELOG_MODIFIED_URL_TEXT +
-          format(OPTIONAL_CHANGELOG_MESSAGE, mr_iid: helper.mr_iid, mr_title: sanitized_mr_title)
+          (helper.ci? ? format(OPTIONAL_CHANGELOG_MESSAGE[:ci], mr_iid: helper.mr_iid, mr_title: sanitized_mr_title) : OPTIONAL_CHANGELOG_MESSAGE[:local])
       end
 
       def required_texts
         required_reasons.each_with_object({}) do |required_reason, memo|
           memo[required_reason] =
             CHANGELOG_MISSING_URL_TEXT +
-              format(REQUIRED_CHANGELOG_MESSAGE, reason: REQUIRED_CHANGELOG_REASONS.fetch(required_reason), mr_iid: helper.mr_iid, mr_title: sanitized_mr_title)
+              (helper.ci? ? format(REQUIRED_CHANGELOG_MESSAGE[:ci], reason: REQUIRED_CHANGELOG_REASONS.fetch(required_reason), mr_iid: helper.mr_iid, mr_title: sanitized_mr_title) : REQUIRED_CHANGELOG_MESSAGE[:local])
         end
       end
 
       def optional_text
         CHANGELOG_MISSING_URL_TEXT +
-          format(OPTIONAL_CHANGELOG_MESSAGE, mr_iid: helper.mr_iid, mr_title: sanitized_mr_title)
+          (helper.ci? ? format(OPTIONAL_CHANGELOG_MESSAGE[:ci], mr_iid: helper.mr_iid, mr_title: sanitized_mr_title) : OPTIONAL_CHANGELOG_MESSAGE[:local])
       end
 
       private

tooling/danger/project_helper.rb

@@ -4,6 +4,7 @@ module Tooling
   module Danger
     module ProjectHelper
       LOCAL_RULES ||= %w[
+        changelog
         changes_size
         commit_messages
         database
@@ -20,7 +21,6 @@ module Tooling
 
       CI_ONLY_RULES ||= %w[
         ce_ee_vue_templates
-        changelog
         ci_templates
         metadata
         feature_flag