From 6c2577eedc6efe5d68c087e0a6929fc6981788a2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc=20Busqu=C3=A9?= Date: Mon, 6 Feb 2023 16:40:33 +0100 Subject: [PATCH 1/2] Remove unused method --- dev_tools/lib/solidus/release_drafter.rb | 4 ---- 1 file changed, 4 deletions(-) diff --git a/dev_tools/lib/solidus/release_drafter.rb b/dev_tools/lib/solidus/release_drafter.rb index 424ce449a3c..eff53fb9125 100644 --- a/dev_tools/lib/solidus/release_drafter.rb +++ b/dev_tools/lib/solidus/release_drafter.rb @@ -44,10 +44,6 @@ def call(pr_number:, current_diff_source_tag:, candidate_tag:, branch:) private - def builder(draft, current_diff_source_tag, candidate_tag) - Builder.new(draft: draft, categories: LABELS.values, prepend: NO_EDIT_WARNING, append: full_changelog(current_diff_source_tag, candidate_tag)) - end - def add_pr(builder, pr, labels) builder.add( number: pr.number, From 51d96f6a91a6dad363b969d765345888cfdbc141 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc=20Busqu=C3=A9?= Date: Mon, 6 Feb 2023 16:50:45 +0100 Subject: [PATCH 2/2] Fix links to PR & author not being added into the automatic Changelog Although GitHub automatically autolinks references to PR and author in the release notes, it doesn't create them for the files in a repository. From theirs docs [1]: > Note: Autolinked references are not created in wikis or files in a repository. As we're copying the automatically generated release notes into the Changelog file, that resulted in the missing links. We fix it from the source to be safe from future changes. [1] - https://docs.github.com/en/get-started/writing-on-github/working-with-advanced-formatting/autolinked-references-and-url Closes #4904 --- dev_tools/lib/solidus/release_drafter.rb | 15 +- .../lib/solidus/release_drafter/builder.rb | 10 +- .../solidus/release_drafter/builder_spec.rb | 131 +++++++++--------- .../spec/lib/solidus/release_drafter_spec.rb | 4 +- 4 files changed, 89 insertions(+), 71 deletions(-) diff --git a/dev_tools/lib/solidus/release_drafter.rb b/dev_tools/lib/solidus/release_drafter.rb index eff53fb9125..a7ffdc54b21 100644 --- a/dev_tools/lib/solidus/release_drafter.rb +++ b/dev_tools/lib/solidus/release_drafter.rb @@ -24,6 +24,8 @@ class ReleaseDrafter TXT + USER_LINK_BUILDER = ->(user) { "https://github.com/#{user}" } + def initialize(github_token:, repository:, client: Client.new(github_token: github_token, repository: repository)) @repository = repository @@ -37,7 +39,14 @@ def call(pr_number:, current_diff_source_tag:, candidate_tag:, branch:) return if pr_labels.include?(SKIP_LABEL) || matching_labels.empty? draft = @client.fetch_draft(tag: candidate_tag) - Builder.new(draft: draft, categories: LABELS.values, prepend: NO_EDIT_WARNING, append: full_changelog(current_diff_source_tag, candidate_tag)) + Builder.new( + draft: draft, + categories: LABELS.values, + prepend: NO_EDIT_WARNING, + append: full_changelog(current_diff_source_tag, candidate_tag), + number_link_builder: number_link_builder, + user_link_builder: USER_LINK_BUILDER + ) .then { |builder| add_pr(builder, pr, matching_labels) } .then { |draft| save_release(draft, candidate_tag, branch) } end @@ -68,5 +77,9 @@ def full_changelog(current_diff_source_tag, candidate_tag) **Full Changelog**: https://github.com/#{@repository}/compare/#{current_diff_source_tag}...#{candidate_tag} TXT end + + def number_link_builder + ->(number) { "https://github.com/#{@repository}/pull/#{number}" } + end end end diff --git a/dev_tools/lib/solidus/release_drafter/builder.rb b/dev_tools/lib/solidus/release_drafter/builder.rb index 78e57147686..bc6d94644ff 100644 --- a/dev_tools/lib/solidus/release_drafter/builder.rb +++ b/dev_tools/lib/solidus/release_drafter/builder.rb @@ -18,16 +18,18 @@ class Builder TITLE_SEPARATOR = SEPARATOR - ENTRY_TEMPLATE = lambda do |title, number, user| - "- #{title} ##{number} (@#{user})" + ENTRY_TEMPLATE = lambda do |title, number, user, number_link_builder, user_link_builder| + "- #{title} [##{number}](#{number_link_builder.(number)}) ([@#{user}](#{user_link_builder.(user)}))" end.freeze ENTRY_SEPARATOR = SEPARATOR SECTION_SEPARATOR = SEPARATOR + SEPARATOR - def initialize(draft:, categories:, prepend: '', append: '') + def initialize(draft:, categories:, number_link_builder:, user_link_builder:, prepend: '', append: '') @categories = Set[*categories] + @number_link_builder = number_link_builder + @user_link_builder = user_link_builder @prepend = prepend @prepend_pattern = /^#{Regexp.quote(@prepend)}/m @append = append @@ -39,7 +41,7 @@ def initialize(draft:, categories:, prepend: '', append: '') end def add(title:, number:, user:, categories:) - ENTRY_TEMPLATE.(title, number, user) + ENTRY_TEMPLATE.(title, number, user, @number_link_builder, @user_link_builder) .then { |entry| add_to_ast(entry, categories) } .then { |ast| @draft.with(content: unparse(ast)) } end diff --git a/dev_tools/spec/lib/solidus/release_drafter/builder_spec.rb b/dev_tools/spec/lib/solidus/release_drafter/builder_spec.rb index ea9a1993630..23b566fae9b 100644 --- a/dev_tools/spec/lib/solidus/release_drafter/builder_spec.rb +++ b/dev_tools/spec/lib/solidus/release_drafter/builder_spec.rb @@ -8,54 +8,57 @@ let(:backend) { 'Solidus Backend' } let(:api) { 'Solidus API' } let(:categories) { [core, backend, api] } + let(:number_link_builder) { ->(number) { "https://github.com/solidusio/solidus/pull/#{number}" } } + let(:user_link_builder) { ->(author) { "https://github.com/#{author}" } } + let(:entry) { ->(title, number, user) { "- #{title} [##{number}](#{number_link_builder.(number)}) ([@#{user}](#{user_link_builder.(user)}))" } } describe '#initialize' do context 'for an existing draft' do it "raises when categories doesn't match" do content = <<~TXT.chomp ## Solidus Core - - Old entry 1 #9 (@bob) + #{entry.('Old entry', 9, 'bob')} ## Solidus Backend TXT draft = Solidus::ReleaseDrafter::Draft.new(url: 'https://release.com', content: content) expect { - described_class.new(draft: draft, categories: categories) + described_class.new(draft: draft, categories: categories, number_link_builder: number_link_builder, user_link_builder: user_link_builder) }.to raise_error(/Given categories don't match those found in the draft/) end it 'raises when prepended text is not present' do content = <<~TXT.chomp ## Solidus Core - - Old entry 1 #9 (@bob) + #{entry.('Old entry 1', 9, 'bob')} ## Solidus Backend ## Solidus API - - Old entry 2 #8 (@alice) + #{entry.('Old entry 2', 8, 'alice')} TXT draft = Solidus::ReleaseDrafter::Draft.new(url: 'https://release.com', content: content) expect { - described_class.new(draft: draft, categories: categories, prepend: "## List of changes\n\n") + described_class.new(draft: draft, categories: categories, prepend: "## List of changes\n\n", number_link_builder: number_link_builder, user_link_builder: user_link_builder) }.to raise_error(/Prepended text is not present in the draft/) end it 'raises when appended text is not present' do content = <<~TXT.chomp ## Solidus Core - - Old entry 1 #9 (@bob) + #{entry.('Old entry 1', 9, 'bob')} ## Solidus Backend ## Solidus API - - Old entry 2 #8 (@alice) + #{entry.('Old entry 2', 8, 'alice')} TXT draft = Solidus::ReleaseDrafter::Draft.new(url: 'https://release.com', content: content) expect { - described_class.new(draft: draft, categories: categories, append: "\n\n That's been everything!") + described_class.new(draft: draft, categories: categories, append: "\n\n That's been everything!", number_link_builder: number_link_builder, user_link_builder: user_link_builder) }.to raise_error(/Appended text is not present in the draft/) end end @@ -66,24 +69,24 @@ let(:draft) { Solidus::ReleaseDrafter::Draft.empty } it 'adds entry under all matching categories' do - builder = described_class.new(draft: draft, categories: categories) + builder = described_class.new(draft: draft, categories: categories, number_link_builder: number_link_builder, user_link_builder: user_link_builder) expect( builder.add(title: 'New entry', number: 10, user: 'alice', categories: [core, api]).content ).to eq <<~TXT.chomp ## Solidus Core - - New entry #10 (@alice) + #{entry.('New entry', 10, 'alice')} ## Solidus Backend ## Solidus API - - New entry #10 (@alice) + #{entry.('New entry', 10, 'alice')} TXT end it "returns the initial draft when categories don't match" do - builder = described_class.new(draft: draft, categories: categories) + builder = described_class.new(draft: draft, categories: categories, number_link_builder: number_link_builder, user_link_builder: user_link_builder) expect( builder.add(title: 'New entry', number: 10, user: 'alice', categories: ['unknown']).content @@ -100,7 +103,7 @@ end it "returns the same when categories are empty" do - builder = described_class.new(draft: draft, categories: categories) + builder = described_class.new(draft: draft, categories: categories, number_link_builder: number_link_builder, user_link_builder: user_link_builder) expect( builder.add(title: 'New entry', number: 10, user: 'alice', categories: []).content @@ -117,7 +120,7 @@ end it 'prepends given text' do - builder = described_class.new(draft: draft, categories: categories, prepend: "## List of changes\n\n") + builder = described_class.new(draft: draft, categories: categories, prepend: "## List of changes\n\n", number_link_builder: number_link_builder, user_link_builder: user_link_builder) expect( builder.add(title: 'New entry', number: 10, user: 'alice', categories: [core, api]).content @@ -125,27 +128,27 @@ ## List of changes ## Solidus Core - - New entry #10 (@alice) + #{entry.('New entry', 10, 'alice')} ## Solidus Backend ## Solidus API - - New entry #10 (@alice) + #{entry.('New entry', 10, 'alice')} TXT end it 'appends given text' do - builder = described_class.new(draft: draft, categories: categories, append: "\n\nThat's been everything!") + builder = described_class.new(draft: draft, categories: categories, append: "\n\nThat's been everything!", number_link_builder: number_link_builder, user_link_builder: user_link_builder) expect( builder.add(title: 'New entry', number: 10, user: 'alice', categories: [core, backend]).content ).to eq <<~TXT.chomp ## Solidus Core - - New entry #10 (@alice) + #{entry.('New entry', 10, 'alice')} ## Solidus Backend - - New entry #10 (@alice) + #{entry.('New entry', 10, 'alice')} ## Solidus API @@ -159,45 +162,45 @@ it 'adds entry under all matching categories' do content = <<~TXT.chomp ## Solidus Core - - Old entry 1 #9 (@bob) + #{entry.('Old entry 1', 9, 'bob')} ## Solidus Backend ## Solidus API - - Old entry 2 #8 (@alice) + #{entry.('Old entry 2', 8, 'bob')} TXT draft = Solidus::ReleaseDrafter::Draft.new(url: 'https://release.com', content: content) - builder = described_class.new(draft: draft, categories: categories) + builder = described_class.new(draft: draft, categories: categories, number_link_builder: number_link_builder, user_link_builder: user_link_builder) expect( builder.add(title: 'New entry', number: 10, user: 'alice', categories: [core, backend]).content ).to eq <<~TXT.chomp ## Solidus Core - - Old entry 1 #9 (@bob) - - New entry #10 (@alice) + #{entry.('Old entry 1', 9, 'bob')} + #{entry.('New entry', 10, 'alice')} ## Solidus Backend - - New entry #10 (@alice) + #{entry.('New entry', 10, 'alice')} ## Solidus API - - Old entry 2 #8 (@alice) + #{entry.('Old entry 2', 8, 'bob')} TXT end it "returns the same when categories don't match" do content = <<~TXT.chomp ## Solidus Core - - Old entry 1 #9 (@bob) + #{entry.('Old entry 1', 9, 'bob')} ## Solidus Backend ## Solidus API - - Old entry 2 #8 (@alice) + #{entry.('Old entry 2', 8, 'alice')} TXT draft = Solidus::ReleaseDrafter::Draft.new(url: 'https://release.com', content: content) - builder = described_class.new(draft: draft, categories: categories) + builder = described_class.new(draft: draft, categories: categories, number_link_builder: number_link_builder, user_link_builder: user_link_builder) expect( builder.add(title: 'New entry', number: 10, user: 'alice', categories: ['unknown']).content @@ -207,16 +210,16 @@ it "returns the same when categories are empty" do content = <<~TXT.chomp ## Solidus Core - - Old entry 1 #9 (@bob) + #{entry.('Old entry 1', 9, 'bob')} ## Solidus Backend ## Solidus API - - Old entry 2 #8 (@alice) + #{entry.('Old entry 2', 8, 'alice')} TXT draft = Solidus::ReleaseDrafter::Draft.new(url: 'https://release.com', content: content) - builder = described_class.new(draft: draft, categories: categories) + builder = described_class.new(draft: draft, categories: categories, number_link_builder: number_link_builder, user_link_builder: user_link_builder) expect( builder.add(title: 'New entry', number: 10, user: 'alice', categories: []).content @@ -228,9 +231,9 @@ ## Solidus Core - - Old entry 1 #9 (@bob) + #{entry.('Old entry 1', 9, 'bob')} - - Old entry 2 #10 (@alice) + #{entry.('Old entry 2', 10, 'alice')} \t ## Solidus Backend @@ -239,18 +242,18 @@ TXT draft = Solidus::ReleaseDrafter::Draft.new(url: 'https://release.com', content: content) - builder = described_class.new(draft: draft, categories: categories) + builder = described_class.new(draft: draft, categories: categories, number_link_builder: number_link_builder, user_link_builder: user_link_builder) expect( builder.add(title: 'New entry', number: 11, user: 'alice', categories: [core, backend]).content ).to eq <<~TXT.chomp ## Solidus Core - - Old entry 1 #9 (@bob) - - Old entry 2 #10 (@alice) - - New entry #11 (@alice) + #{entry.('Old entry 1', 9, 'bob')} + #{entry.('Old entry 2', 10, 'alice')} + #{entry.('New entry', 11, 'alice')} ## Solidus Backend - - New entry #11 (@alice) + #{entry.('New entry', 11, 'alice')} ## Solidus API @@ -261,60 +264,60 @@ content = <<~TXT.chomp Outlier ## Solidus Core - - Old entry 1 #9 (@bob) + #{entry.('Old entry 1', 9, 'bob')} ## Solidus Backend ## Solidus API - - Old entry 2 #8 (@alice) + #{entry.('Old entry 2', 8, 'alice')} TXT draft = Solidus::ReleaseDrafter::Draft.new(url: 'https://release.com', content: content) - builder = described_class.new(draft: draft, categories: categories) + builder = described_class.new(draft: draft, categories: categories, number_link_builder: number_link_builder, user_link_builder: user_link_builder) expect( builder.add(title: 'New entry', number: 10, user: 'alice', categories: [core]).content ).to eq <<~TXT.chomp ## Solidus Core - - Old entry 1 #9 (@bob) - - New entry #10 (@alice) + #{entry.('Old entry 1', 9, 'bob')} + #{entry.('New entry', 10, 'alice')} ## Solidus Backend ## Solidus API - - Old entry 2 #8 (@alice) + #{entry.('Old entry 2', 8, 'alice')} TXT end it 'keeps unknown content within a category' do content = <<~TXT.chomp ## Solidus Core - - Old entry 1 #9 (@bob) + #{entry.('Old entry 1', 9, 'bob')} Outlier ## Solidus Backend ## Solidus API - - Old entry 2 #8 (@alice) + #{entry.('Old entry 2', 8, 'alice')} TXT draft = Solidus::ReleaseDrafter::Draft.new(url: 'https://release.com', content: content) - builder = described_class.new(draft: draft, categories: categories) + builder = described_class.new(draft: draft, categories: categories, number_link_builder: number_link_builder, user_link_builder: user_link_builder) expect( builder.add(title: 'New entry', number: 10, user: 'alice', categories: [core]).content ).to eq <<~TXT.chomp ## Solidus Core - - Old entry 1 #9 (@bob) + #{entry.('Old entry 1', 9, 'bob')} Outlier - - New entry #10 (@alice) + #{entry.('New entry', 10, 'alice')} ## Solidus Backend ## Solidus API - - Old entry 2 #8 (@alice) + #{entry.('Old entry 2', 8, 'alice')} TXT end @@ -323,15 +326,15 @@ ## List of changes ## Solidus Core - - Old entry 1 #9 (@bob) + #{entry.('Old entry 1', 9, 'bob')} ## Solidus Backend ## Solidus API - - Old entry 2 #8 (@alice) + #{entry.('Old entry 2', 8, 'alice')} TXT draft = Solidus::ReleaseDrafter::Draft.new(url: 'https://release.com', content: content) - builder = described_class.new(draft: draft, categories: categories, prepend: "## List of changes\n\n") + builder = described_class.new(draft: draft, categories: categories, prepend: "## List of changes\n\n", number_link_builder: number_link_builder, user_link_builder: user_link_builder) expect( builder.add(title: 'New entry', number: 10, user: 'alice', categories: [core, api]).content @@ -339,44 +342,44 @@ ## List of changes ## Solidus Core - - Old entry 1 #9 (@bob) - - New entry #10 (@alice) + #{entry.('Old entry 1', 9, 'bob')} + #{entry.('New entry', 10, 'alice')} ## Solidus Backend ## Solidus API - - Old entry 2 #8 (@alice) - - New entry #10 (@alice) + #{entry.('Old entry 2', 8, 'alice')} + #{entry.('New entry', 10, 'alice')} TXT end it 'keeps appended text in place' do content = <<~TXT.chomp ## Solidus Core - - Old entry 1 #9 (@bob) + #{entry.('Old entry 1', 9, 'bob')} ## Solidus Backend ## Solidus API - - Old entry 2 #8 (@alice) + #{entry.('Old entry 2', 8, 'alice')} That's been everything! TXT draft = Solidus::ReleaseDrafter::Draft.new(url: 'https://release.com', content: content) - builder = described_class.new(draft: draft, categories: categories, append: "\n\nThat's been everything!") + builder = described_class.new(draft: draft, categories: categories, append: "\n\nThat's been everything!", number_link_builder: number_link_builder, user_link_builder: user_link_builder) expect( builder.add(title: 'New entry', number: 10, user: 'alice', categories: [backend]).content ).to eq <<~TXT.chomp ## Solidus Core - - Old entry 1 #9 (@bob) + #{entry.('Old entry 1', 9, 'bob')} ## Solidus Backend - - New entry #10 (@alice) + #{entry.('New entry', 10, 'alice')} ## Solidus API - - Old entry 2 #8 (@alice) + #{entry.('Old entry 2', 8, 'alice')} That's been everything! TXT diff --git a/dev_tools/spec/lib/solidus/release_drafter_spec.rb b/dev_tools/spec/lib/solidus/release_drafter_spec.rb index 9f8a4ef073c..60dd7935f1a 100644 --- a/dev_tools/spec/lib/solidus/release_drafter_spec.rb +++ b/dev_tools/spec/lib/solidus/release_drafter_spec.rb @@ -51,7 +51,7 @@ def add_pr(pr_number:, labels: []) expect( subject.call(pr_number: 1, current_diff_source_tag: 'v3.0.0', candidate_tag: 'v3.1.0', branch: 'master').content - ).to match(/#{Regexp.escape("## Solidus Core\n- PR number 1 #1 (@alice)")}/) + ).to match(/#{Regexp.escape("## Solidus Core\n- PR number 1 [#1](https://github.com/fake/solidus/pull/1) ([@alice](https://github.com/alice))")}/) end it 'adds the PR under more than one matching label' do @@ -59,7 +59,7 @@ def add_pr(pr_number:, labels: []) expect( subject.call(pr_number: 1, current_diff_source_tag: 'v3.0.0', candidate_tag: 'v3.1.0', branch: 'master').content - ).to match(/#{Regexp.escape("## Solidus Core\n- PR number 1 #1 (@alice)")}.*#{Regexp.escape("## Solidus API\n- PR number 1 #1 (@alice)")}/m) + ).to match(/#{Regexp.escape("## Solidus Core\n- PR number 1 [#1](https://github.com/fake/solidus/pull/1) ([@alice](https://github.com/alice))")}.*#{Regexp.escape("## Solidus API\n- PR number 1 [#1](https://github.com/fake/solidus/pull/1) ([@alice](https://github.com/alice))")}/m) end it 'ignores if no matching label is present' do