From 17e9448a05d96369a46c7be89ad2cd45be0150f0 Mon Sep 17 00:00:00 2001 From: Asherah Connor Date: Fri, 12 Dec 2025 13:32:00 +1100 Subject: [PATCH 1/5] Refactor documentation Dangerfile into plugin --- danger/documentation/Dangerfile | 126 +--------------- danger/plugins/documentation.rb | 9 ++ spec/tooling/danger/documentation_spec.rb | 52 +++++++ tooling/danger/documentation.rb | 170 ++++++++++++++++++++++ 4 files changed, 232 insertions(+), 125 deletions(-) create mode 100644 danger/plugins/documentation.rb create mode 100644 spec/tooling/danger/documentation_spec.rb create mode 100644 tooling/danger/documentation.rb diff --git a/danger/documentation/Dangerfile b/danger/documentation/Dangerfile index 87dab4b5838d98..8afd1bcfbb548d 100644 --- a/danger/documentation/Dangerfile +++ b/danger/documentation/Dangerfile @@ -1,127 +1,3 @@ # frozen_string_literal: true -def feature_mr? - (helper.mr_labels & %w[feature::addition feature::enhancement]).any? -end - -def doc_path_to_url(path) - path.sub("doc/", "https://docs.gitlab.com/").sub("index.md", "").sub(".md", "/") -end - -docs_paths_to_review = helper.changes_by_category[:docs] - -# Some docs do not need a review from a Technical Writer. -# In these cases, we'll output a message specific to the section. -sections_with_no_tw_review = { - 'doc/architecture' => [], - 'doc/development' => [], - 'doc/solutions' => [], - 'doc-locale' => [] -}.freeze - -# One exception to the exceptions above: Technical Writing docs should get a TW review. -TW_DOCS_PATH = 'doc/development/documentation' - -# Some docs require a longer pipeline, which cannot be avoided. -sections_with_tier3_review = { - 'doc/_index.md' => [], - 'doc/api/settings.md' => [] -} - -docs_paths_to_review.reject! do |doc| - section_with_no_tw_review = sections_with_no_tw_review.keys.find { |skip_path| doc.start_with?(skip_path) && !doc.start_with?(TW_DOCS_PATH) } - next unless section_with_no_tw_review - - sections_with_no_tw_review[section_with_no_tw_review] << doc - - true -end - -docs_paths_to_review.reject! do |doc| - section = sections_with_tier3_review.keys.find { |skip_path| doc.start_with?(skip_path) && !doc.start_with?(TW_DOCS_PATH) } - next unless section - - sections_with_tier3_review[section] << doc - - true -end - -SOLUTIONS_LABELS = %w[Solutions].freeze -DEVELOPMENT_LABELS = ['development guidelines'].freeze - -def add_labels(labels) - helper.labels_to_add.concat(%w[documentation type::maintenance maintenance::refactor] + labels) -end - -ANY_MAINTAINER_CAN_MERGE_MESSAGE = <<~MSG -This MR contains docs in the /%s directory, but any Maintainer can merge. You do not need tech writer review. -MSG - -SOLUTIONS_MESSAGE = <<~MSG -This MR contains docs in the /doc/solutions directory and should be reviewed by a Solutions Architect approver. You do not need tech writer review. -MSG - -LOCALIZATION_MESSAGE = <<~MSG -This MR contains files in the /doc-locale directory. These files are translations maintained through a separate process and should not be edited directly. If you are not part of the Localization team, please remove the changes to these files from your MR. -MSG - -DOCS_LONG_PIPELINE_MESSAGE = <<~MSG -This merge request contains documentation files that require a tier-3 code pipeline before merge. After you complete all needed documentation reviews with short docs pipelines, see the [instructions for running a long pipeline](https://docs.gitlab.com/development/documentation/workflow/#pipelines-and-branch-naming) to this merge request. -MSG - -# For regular pages, prompt for a TW review -DOCS_UPDATE_SHORT_MESSAGE = <<~MSG -This merge request adds or changes documentation files and requires Technical Writing review. The review should happen before merge, but can be post-merge if the merge request is time sensitive. -MSG - -DOCS_UPDATE_LONG_MESSAGE = <<~MSG.freeze -## Documentation review - -The following files require a review from a technical writer: - -* #{docs_paths_to_review.map { |path| "`#{path}` ([Link to current live version](#{doc_path_to_url(path)}))" }.join("\n* ")} - -The review does not need to block merging this merge request. See the: - -- [Metadata for the `*.md` files](https://docs.gitlab.com/ee/development/documentation/#metadata) that you've changed. The first few lines of each `*.md` file identify the stage and group most closely associated with your docs change. -- The [Technical Writer assigned](https://handbook.gitlab.com/handbook/product/ux/technical-writing/#assignments) for that stage and group. -- [Documentation workflows](https://docs.gitlab.com/development/documentation/workflow/) for information on when to assign a merge request for review. -MSG - -# Documentation should be updated for feature::addition and feature::enhancement -DOCUMENTATION_UPDATE_MISSING = <<~MSG -~"feature::addition" and ~"feature::enhancement" merge requests normally have a documentation change. Consider adding a documentation update or confirming the documentation plan with the [Technical Writer counterpart](https://handbook.gitlab.com/handbook/product/ux/technical-writing/#assignments). - -For more information, see: - -- The Handbook page on [merge request types](https://handbook.gitlab.com/handbook/product/groups/product-analysis/engineering/metrics/#work-type-classification). -- The [definition of done](https://docs.gitlab.com/development/contributing/merge_request_workflow/#definition-of-done) documentation. -MSG - -# Output messages -warn(DOCUMENTATION_UPDATE_MISSING) if docs_paths_to_review.empty? && feature_mr? - -if sections_with_no_tw_review["doc/architecture"].any? - message(format(ANY_MAINTAINER_CAN_MERGE_MESSAGE, directory: 'doc/architecture')) -end - -if sections_with_no_tw_review["doc/development"].any? - add_labels(DEVELOPMENT_LABELS) - message(format(ANY_MAINTAINER_CAN_MERGE_MESSAGE, directory: 'doc/development')) -end - -if sections_with_no_tw_review["doc/solutions"].any? - add_labels(SOLUTIONS_LABELS) - message(SOLUTIONS_MESSAGE) -end - -if sections_with_no_tw_review["doc-locale"].any? - message(LOCALIZATION_MESSAGE, file: sections_with_no_tw_review["doc-locale"].first, line: 1) -end - -message(DOCS_LONG_PIPELINE_MESSAGE) if sections_with_tier3_review.values.flatten.any? - -unless docs_paths_to_review.empty? - message(DOCS_UPDATE_SHORT_MESSAGE) - markdown(DOCS_UPDATE_LONG_MESSAGE) -end +documentation.check_documentation diff --git a/danger/plugins/documentation.rb b/danger/plugins/documentation.rb new file mode 100644 index 00000000000000..00e901358a4684 --- /dev/null +++ b/danger/plugins/documentation.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +require_relative '../../tooling/danger/documentation' + +module Danger + class Documentation < ::Danger::Plugin + include Tooling::Danger::Documentation + end +end diff --git a/spec/tooling/danger/documentation_spec.rb b/spec/tooling/danger/documentation_spec.rb new file mode 100644 index 00000000000000..d91943b450b783 --- /dev/null +++ b/spec/tooling/danger/documentation_spec.rb @@ -0,0 +1,52 @@ +# frozen_string_literal: true + +require 'fast_spec_helper' +require 'gitlab/dangerfiles/spec_helper' +require_relative '../../../tooling/danger/documentation' +require_relative '../../../tooling/danger/project_helper' + +RSpec.describe Tooling::Danger::Documentation, feature_category: :markdown do + include_context "with dangerfile" + + subject(:documentation) { fake_danger.new(helper: fake_helper) } + + let(:fake_danger) { DangerSpecHelper.fake_danger.include(described_class) } + let(:mr_labels) { [] } + + before do + allow(fake_helper).to receive(:changes_by_category).and_return({ docs:, mr_labels: }) + end + + describe '#check_documentation' do + subject(:check_documentation) { documentation.check_documentation } + + shared_examples_for 'nothing to report' do + it 'does not add any message or warning' do + expect(documentation).not_to receive(:warn) + expect(documentation).not_to receive(:message) + check_documentation + end + end + + shared_examples_for 'warns' do |warning| + it 'adds a warning' do + expect(documentation).to receive(:warn).with(warning) + expect(documentation).not_to receive(:message) + check_documentation + end + end + + context 'when there are no documentation changes' do + let(:docs) { [] } + + it_behaves_like 'nothing to report' + end + + context 'when there are no documentation changes on a feature MR' do + let(:docs) { [] } + let(:mr_labels) { %w[feature::addition] } + + it_behaves_like 'warns', described_class::DOCUMENTATION_UPDATE_MISSING + end + end +end diff --git a/tooling/danger/documentation.rb b/tooling/danger/documentation.rb new file mode 100644 index 00000000000000..dcdf546f680893 --- /dev/null +++ b/tooling/danger/documentation.rb @@ -0,0 +1,170 @@ +# frozen_string_literal: true + +module Tooling + module Danger + module Documentation + SOLUTIONS_LABELS = %w[Solutions].freeze + DEVELOPMENT_LABELS = ['development guidelines'].freeze + + ANY_MAINTAINER_CAN_MERGE_MESSAGE_TEMPLATE = <<~MSG + This MR contains docs in the /%s directory, but any Maintainer can merge. + You do not need tech writer review. + MSG + + SOLUTIONS_MESSAGE = <<~MSG + This MR contains docs in the /doc/solutions directory and should be reviewed by a Solutions Architect approver. + You do not need tech writer review. + MSG + + LOCALIZATION_MESSAGE = <<~MSG + This MR contains files in the /doc-locale directory. These files are translations maintained through a separate + process and should not be edited directly. If you are not part of the Localization team, please remove the + changes to these files from your MR. + MSG + + DOCS_LONG_PIPELINE_MESSAGE = <<~MSG + This merge request contains documentation files that require a tier-3 code pipeline before merge. After you + complete all needed documentation reviews with short docs pipelines, see the [instructions for running a long + pipeline](https://docs.gitlab.com/development/documentation/workflow/#pipelines-and-branch-naming) to this merge + request. + MSG + + # For regular pages, prompt for a TW review + DOCS_UPDATE_SHORT_MESSAGE = <<~MSG + This merge request adds or changes documentation files and requires Technical Writing review. The review should + happen before merge, but can be post-merge if the merge request is time sensitive. + MSG + + DOCS_UPDATE_LONG_MESSAGE_TEMPLATE = <<~MSG + ## Documentation review + + The following files require a review from a technical writer: + + %s + + The review does not need to block merging this merge request. See the: + + - [Metadata for the `*.md` files](https://docs.gitlab.com/ee/development/documentation/#metadata) that you've + changed. The first few lines of each `*.md` file identify the stage and group most closely associated with + your docs change. + - The + [Technical Writer assigned](https://handbook.gitlab.com/handbook/product/ux/technical-writing/#assignments) + for that stage and group. + - [Documentation workflows](https://docs.gitlab.com/development/documentation/workflow/) for information on when + to assign a merge request for review. + MSG + + # Documentation should be updated for feature::addition and feature::enhancement + DOCUMENTATION_UPDATE_MISSING = <<~MSG + ~"feature::addition" and ~"feature::enhancement" merge requests normally have a documentation change. Consider + adding a documentation update or confirming the documentation plan with the + [Technical Writer counterpart](https://handbook.gitlab.com/handbook/product/ux/technical-writing/#assignments). + + For more information, see: + + - The Handbook page on + [merge request types](https://handbook.gitlab.com/handbook/product/groups/product-analysis/engineering/metrics/#work-type-classification). + - The + [definition of done](https://docs.gitlab.com/development/contributing/merge_request_workflow/#definition-of-done) + documentation. + MSG + + # Some docs do not need a review from a Technical Writer. + # In these cases, we'll output a message specific to the section. + SECTIONS_WITH_NO_TW_REVIEW = [ + 'doc/architecture', + 'doc/development', + 'doc/solutions', + 'doc-locale' + ].freeze + + # Some docs require a longer pipeline, which cannot be avoided. + SECTIONS_WITH_TIER3_REVIEW = [ + 'doc/_index.md', + 'doc/api/settings.md' + ].freeze + + # One exception to the exceptions above: Technical Writing docs should get a TW review. + TW_DOCS_PATH = 'doc/development/documentation' + + def check_documentation + # Output messages + warn(DOCUMENTATION_UPDATE_MISSING) if docs_paths_to_review.empty? && feature_mr? + + if sections_with_no_tw_review["doc/architecture"].any? + message(format(ANY_MAINTAINER_CAN_MERGE_MESSAGE_TEMPLATE, directory: 'doc/architecture')) + end + + if sections_with_no_tw_review["doc/development"].any? + add_labels(DEVELOPMENT_LABELS) + message(format(ANY_MAINTAINER_CAN_MERGE_MESSAGE_TEMPLATE, directory: 'doc/development')) + end + + if sections_with_no_tw_review["doc/solutions"].any? + add_labels(SOLUTIONS_LABELS) + message(SOLUTIONS_MESSAGE) + end + + if sections_with_no_tw_review["doc-locale"].any? + message(LOCALIZATION_MESSAGE, file: sections_with_no_tw_review["doc-locale"].first, line: 1) + end + + message(DOCS_LONG_PIPELINE_MESSAGE) if sections_with_tier3_review.values.flatten.any? + + return if docs_paths_to_review.empty? + + message(DOCS_UPDATE_SHORT_MESSAGE) + markdown(format( + DOCS_UPDATE_LONG_MESSAGE_TEMPLATE, + doc_paths_to_review: + docs_paths_to_review.map do |path| + "* `#{path}` ([Link to current live version](#{doc_path_to_url(path)}))" + end.join("\n") + )) + end + + private + + def docs_changes + helper.changes_by_category[:docs] + end + + def group_docs_by_sections(sections) + grouped = Hash.new { [] } + + docs_changes.each do |doc| + next if doc.start_with?(TW_DOCS_PATH) + + section = sections.find { |prefix| doc.start_with?(prefix) } + grouped[section] = grouped[section] << doc if section + end + + grouped + end + + def docs_paths_to_review + @docs_paths_to_review ||= docs_changes - sections_with_no_tw_review.values - sections_with_tier3_review.values + end + + def sections_with_no_tw_review + @sections_with_no_tw_review ||= group_docs_by_sections(SECTIONS_WITH_NO_TW_REVIEW) + end + + def sections_with_tier3_review + @sections_with_tier3_review ||= group_docs_by_sections(SECTIONS_WITH_TIER3_REVIEW) + end + + def feature_mr? + (helper.mr_labels & %w[feature::addition feature::enhancement]).any? + end + + def doc_path_to_url(path) + path.sub("doc/", "https://docs.gitlab.com/").sub("index.md", "").sub(".md", "/") + end + + def add_labels(labels) + helper.labels_to_add.concat(%w[documentation type::maintenance maintenance::refactor] + labels) + end + end + end +end -- GitLab From 224694368f458f7d7e8b2eb34b471361228af113 Mon Sep 17 00:00:00 2001 From: Asherah Connor Date: Fri, 12 Dec 2025 14:26:23 +1100 Subject: [PATCH 2/5] doc/architecture no longer exists; ensure subpath match --- tooling/danger/documentation.rb | 21 ++++++++------------- 1 file changed, 8 insertions(+), 13 deletions(-) diff --git a/tooling/danger/documentation.rb b/tooling/danger/documentation.rb index dcdf546f680893..bcfa8654754084 100644 --- a/tooling/danger/documentation.rb +++ b/tooling/danger/documentation.rb @@ -72,10 +72,9 @@ module Documentation # Some docs do not need a review from a Technical Writer. # In these cases, we'll output a message specific to the section. SECTIONS_WITH_NO_TW_REVIEW = [ - 'doc/architecture', - 'doc/development', - 'doc/solutions', - 'doc-locale' + 'doc/development/', + 'doc/solutions/', + 'doc-locale/' ].freeze # Some docs require a longer pipeline, which cannot be avoided. @@ -91,22 +90,18 @@ def check_documentation # Output messages warn(DOCUMENTATION_UPDATE_MISSING) if docs_paths_to_review.empty? && feature_mr? - if sections_with_no_tw_review["doc/architecture"].any? - message(format(ANY_MAINTAINER_CAN_MERGE_MESSAGE_TEMPLATE, directory: 'doc/architecture')) - end - - if sections_with_no_tw_review["doc/development"].any? + if sections_with_no_tw_review["doc/development/"].any? add_labels(DEVELOPMENT_LABELS) - message(format(ANY_MAINTAINER_CAN_MERGE_MESSAGE_TEMPLATE, directory: 'doc/development')) + message(format(ANY_MAINTAINER_CAN_MERGE_MESSAGE_TEMPLATE, directory: 'doc/development/')) end - if sections_with_no_tw_review["doc/solutions"].any? + if sections_with_no_tw_review["doc/solutions/"].any? add_labels(SOLUTIONS_LABELS) message(SOLUTIONS_MESSAGE) end - if sections_with_no_tw_review["doc-locale"].any? - message(LOCALIZATION_MESSAGE, file: sections_with_no_tw_review["doc-locale"].first, line: 1) + if sections_with_no_tw_review["doc-locale/"].any? + message(LOCALIZATION_MESSAGE, file: sections_with_no_tw_review["doc-locale/"].first, line: 1) end message(DOCS_LONG_PIPELINE_MESSAGE) if sections_with_tier3_review.values.flatten.any? -- GitLab From c5898a75b15271bc1186bb9bfb6dc6fecd3c06f3 Mon Sep 17 00:00:00 2001 From: Asherah Connor Date: Fri, 12 Dec 2025 14:33:34 +1100 Subject: [PATCH 3/5] Expand specs, fix handling of _index.md --- spec/tooling/danger/documentation_spec.rb | 145 +++++++++++++++++++++- tooling/danger/documentation.rb | 8 +- 2 files changed, 146 insertions(+), 7 deletions(-) diff --git a/spec/tooling/danger/documentation_spec.rb b/spec/tooling/danger/documentation_spec.rb index d91943b450b783..958a8ad58cfadf 100644 --- a/spec/tooling/danger/documentation_spec.rb +++ b/spec/tooling/danger/documentation_spec.rb @@ -12,26 +12,85 @@ let(:fake_danger) { DangerSpecHelper.fake_danger.include(described_class) } let(:mr_labels) { [] } + let(:labels_to_add) { [] } + let(:existing_scoped_label_scopes) { [] } before do - allow(fake_helper).to receive(:changes_by_category).and_return({ docs:, mr_labels: }) + allow(fake_helper).to receive_messages( + changes_by_category: { docs:, mr_labels: }, + labels_to_add: labels_to_add + ) + allow(fake_helper).to receive(:has_scoped_label_with_scope?) do |scope| + existing_scoped_label_scopes.include?(scope) + end + allow(documentation).to receive(:message) + allow(documentation).to receive(:markdown) end describe '#check_documentation' do subject(:check_documentation) { documentation.check_documentation } - shared_examples_for 'nothing to report' do - it 'does not add any message or warning' do + shared_examples_for "doesn't warn" do + it 'does not add a warning' do expect(documentation).not_to receive(:warn) + check_documentation + end + end + + shared_examples_for "doesn't message" do + it 'does not add a message' do expect(documentation).not_to receive(:message) check_documentation end end + shared_examples_for "doesn't add labels" do + it 'does not add labels' do + check_documentation + expect(labels_to_add).to be_empty + end + end + + shared_examples_for "doesn't add markdown" do + it 'does not add markdown' do + expect(documentation).not_to receive(:markdown) + check_documentation + end + end + shared_examples_for 'warns' do |warning| it 'adds a warning' do expect(documentation).to receive(:warn).with(warning) expect(documentation).not_to receive(:message) + expect(documentation).not_to receive(:markdown) + check_documentation + end + end + + shared_examples_for 'adds messages' do |*expectations| + it 'adds a message' do + expect(documentation).not_to receive(:warn) + messages = [] + expect(documentation).to receive(:message) do |actual, *_| + messages << actual + end + check_documentation + messages.zip(expectations) do |actual, expected| + expect(actual).to match(expected) + end + end + end + + shared_examples_for 'adds labels' do |labels| + it "adds labels #{labels.join(', ')}" do + check_documentation + expect(labels_to_add.uniq).to match_array(labels) + end + end + + shared_examples_for 'adds markdown' do |markdown| + it "adds #{markdown.inspect} to markdown" do + expect(documentation).to receive(:markdown).with(markdown) check_documentation end end @@ -39,7 +98,10 @@ context 'when there are no documentation changes' do let(:docs) { [] } - it_behaves_like 'nothing to report' + it_behaves_like "doesn't warn" + it_behaves_like "doesn't message" + it_behaves_like "doesn't add labels" + it_behaves_like "doesn't add markdown" end context 'when there are no documentation changes on a feature MR' do @@ -47,6 +109,81 @@ let(:mr_labels) { %w[feature::addition] } it_behaves_like 'warns', described_class::DOCUMENTATION_UPDATE_MISSING + it_behaves_like "doesn't message" + it_behaves_like "doesn't add labels" + it_behaves_like "doesn't add markdown" + end + + context 'when there are development docs changes' do + let(:docs) { ['doc/development/logs.md', 'doc/development/semver.md'] } + + it_behaves_like "doesn't warn" + it_behaves_like 'adds messages', %r{contains docs in the /doc/development directory, but any Maintainer can merge} + it_behaves_like 'adds labels', + ['development guidelines', 'documentation', 'type::maintenance', 'maintenance::refactor'] + it_behaves_like "doesn't add markdown" + end + + context 'when there are solutions docs changes' do + let(:docs) { ['doc/solutions/_index.md'] } + + it_behaves_like "doesn't warn" + it_behaves_like 'adds messages', + %r{contains docs in the /doc/solutions directory and should be reviewed by a Solutions Architect} + it_behaves_like 'adds labels', + ['Solutions', 'documentation', 'type::maintenance', 'maintenance::refactor'] + it_behaves_like "doesn't add markdown" + end + + context 'when there are localized doc changes' do + let(:docs) { ['doc-locale/ja-jp/_index.md', 'doc-locale/ja-jp/devsecops.md'] } + + it_behaves_like "doesn't warn" + it_behaves_like 'adds messages', %r{should not be edited directly} + it_behaves_like "doesn't add labels" + it_behaves_like "doesn't add markdown" + end + + context 'when there are docs changes requiring the tier 3 pipeline' do + let(:docs) { ['doc/api/settings.md'] } + + it_behaves_like "doesn't warn" + it_behaves_like 'adds messages', %r{that require a tier-3 code pipeline} + it_behaves_like "doesn't add labels" + it_behaves_like "doesn't add markdown" + end + + context 'when there are other docs changes' do + let(:docs) { ['doc/user/markdown.md', 'doc/security/_index.md'] } + + it_behaves_like "doesn't warn" + it_behaves_like 'adds messages', %r{adds or changes documentation files and requires Technical Writing review} + it_behaves_like "doesn't add labels" + it_behaves_like 'adds markdown', + /#{Regexp.escape "* `doc/user/markdown.md` ([Link to current live version](https://docs.gitlab.com/user/markdown/))\n"}/ + it_behaves_like 'adds markdown', + /#{Regexp.escape "* `doc/security/_index.md` ([Link to current live version](https://docs.gitlab.com/security/))\n"}/ + end + + context 'when there are many kinds of doc changes' do + let(:docs) do + ['doc/development/mcp_server.md', 'doc/solutions/cloud/_index.md', 'doc/_index.md', + 'doc/legal/developer_certificate_of_origin.md', 'doc/ci/debugging.md'] + end + + it_behaves_like "doesn't warn" + it_behaves_like 'adds messages', + %r{contains docs in the /doc/development directory, but any Maintainer can merge}, + %r{contains docs in the /doc/solutions directory and should be reviewed by a Solutions Architect}, + %r{that require a tier-3 code pipeline} + it_behaves_like 'adds labels', + ['development guidelines', 'Solutions', 'documentation', 'type::maintenance', + 'maintenance::refactor'] + it_behaves_like 'adds markdown', + /#{Regexp.escape "* `doc/legal/developer_certificate_of_origin.md` ([Link to current live version](https://docs.gitlab.com/legal/developer_certificate_of_origin/))\n"}/ + it_behaves_like 'adds markdown', + /#{Regexp.escape "* `doc/ci/debugging.md` ([Link to current live version](https://docs.gitlab.com/ci/debugging/))\n"}/ + end end end end diff --git a/tooling/danger/documentation.rb b/tooling/danger/documentation.rb index bcfa8654754084..dec5c1970b0d63 100644 --- a/tooling/danger/documentation.rb +++ b/tooling/danger/documentation.rb @@ -92,7 +92,7 @@ def check_documentation if sections_with_no_tw_review["doc/development/"].any? add_labels(DEVELOPMENT_LABELS) - message(format(ANY_MAINTAINER_CAN_MERGE_MESSAGE_TEMPLATE, directory: 'doc/development/')) + message(format(ANY_MAINTAINER_CAN_MERGE_MESSAGE_TEMPLATE, directory: 'doc/development')) end if sections_with_no_tw_review["doc/solutions/"].any? @@ -138,7 +138,9 @@ def group_docs_by_sections(sections) end def docs_paths_to_review - @docs_paths_to_review ||= docs_changes - sections_with_no_tw_review.values - sections_with_tier3_review.values + @docs_paths_to_review ||= docs_changes - + sections_with_no_tw_review.values.flatten - + sections_with_tier3_review.values.flatten end def sections_with_no_tw_review @@ -154,7 +156,7 @@ def feature_mr? end def doc_path_to_url(path) - path.sub("doc/", "https://docs.gitlab.com/").sub("index.md", "").sub(".md", "/") + path.sub(%r{\Adoc/}, "https://docs.gitlab.com/").sub(%r{/_index\.md\z}, "/").sub(%r{\.md\z}, "/") end def add_labels(labels) -- GitLab From 3a83b13d3d0c66fa7e990e6fccc0ba9c4edd631d Mon Sep 17 00:00:00 2001 From: Asherah Connor Date: Fri, 12 Dec 2025 15:34:57 +1100 Subject: [PATCH 4/5] Fix ~type fighting --- spec/tooling/danger/documentation_spec.rb | 6 ++++++ tooling/danger/documentation.rb | 5 ++++- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/spec/tooling/danger/documentation_spec.rb b/spec/tooling/danger/documentation_spec.rb index 958a8ad58cfadf..fe8d1ba5af2876 100644 --- a/spec/tooling/danger/documentation_spec.rb +++ b/spec/tooling/danger/documentation_spec.rb @@ -184,6 +184,12 @@ it_behaves_like 'adds markdown', /#{Regexp.escape "* `doc/ci/debugging.md` ([Link to current live version](https://docs.gitlab.com/ci/debugging/))\n"}/ end + + context 'when there are development docs changes and the MR already has a type label' do + let(:docs) { ['doc/development/json.md'] } + let(:existing_scoped_label_scopes) { ['type'] } + + it_behaves_like 'adds labels', ['development guidelines', 'documentation', 'maintenance::refactor'] end end end diff --git a/tooling/danger/documentation.rb b/tooling/danger/documentation.rb index dec5c1970b0d63..f4b8658493eb73 100644 --- a/tooling/danger/documentation.rb +++ b/tooling/danger/documentation.rb @@ -160,7 +160,10 @@ def doc_path_to_url(path) end def add_labels(labels) - helper.labels_to_add.concat(%w[documentation type::maintenance maintenance::refactor] + labels) + helper.labels_to_add.concat(labels) + helper.labels_to_add.push('documentation') + helper.labels_to_add.push('type::maintenance') unless helper.has_scoped_label_with_scope?('type') + helper.labels_to_add.push('maintenance::refactor') unless helper.has_scoped_label_with_scope?('maintenance') end end end -- GitLab From 0dcca6e82bb9294b02757b3d6e7756df6ee41652 Mon Sep 17 00:00:00 2001 From: Asherah Connor Date: Fri, 12 Dec 2025 16:13:05 +1100 Subject: [PATCH 5/5] Use a more common idiom --- tooling/danger/documentation.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tooling/danger/documentation.rb b/tooling/danger/documentation.rb index f4b8658493eb73..054ac028643677 100644 --- a/tooling/danger/documentation.rb +++ b/tooling/danger/documentation.rb @@ -125,13 +125,13 @@ def docs_changes end def group_docs_by_sections(sections) - grouped = Hash.new { [] } + grouped = Hash.new { |h, k| h[k] = [] } docs_changes.each do |doc| next if doc.start_with?(TW_DOCS_PATH) section = sections.find { |prefix| doc.start_with?(prefix) } - grouped[section] = grouped[section] << doc if section + grouped[section] << doc if section end grouped -- GitLab