diff --git a/danger/documentation/Dangerfile b/danger/documentation/Dangerfile index 87dab4b5838d98b5f553ad18e7373b01ef6194de..8afd1bcfbb548d5f68c30e227ae9fc4bdf9a2df7 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 0000000000000000000000000000000000000000..00e901358a468448b705c3c76995f7675fa32a75 --- /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 0000000000000000000000000000000000000000..fe8d1ba5af28768e25ee22ee37b7b1b926a60b44 --- /dev/null +++ b/spec/tooling/danger/documentation_spec.rb @@ -0,0 +1,195 @@ +# 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) { [] } + let(:labels_to_add) { [] } + let(:existing_scoped_label_scopes) { [] } + + before do + 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 "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 + + context 'when there are no documentation changes' do + let(:docs) { [] } + + 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 + let(:docs) { [] } + 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 + + 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 new file mode 100644 index 0000000000000000000000000000000000000000..054ac0286436776f2e51d6286008909b9fc2b481 --- /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/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/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 { |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] << doc if section + end + + grouped + end + + def docs_paths_to_review + @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 + @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(%r{\Adoc/}, "https://docs.gitlab.com/").sub(%r{/_index\.md\z}, "/").sub(%r{\.md\z}, "/") + end + + def add_labels(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 +end