From 065a19e9d8f7d51e5801192cab7eb2252b392c5f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc=20Busqu=C3=A9?= Date: Thu, 9 Feb 2023 06:16:44 +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 1879d0d6074212fd48524403c5b2162e6fa1be96 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc=20Busqu=C3=A9?= Date: Thu, 9 Feb 2023 06:28:52 +0100 Subject: [PATCH 2/2] Use the same format as GitHub's default for the release notes We're changing the format for the auto-generated release notes from: - This is the PR title #1 (@author) to: * This is the PR title by @author in https://github.com/solidusio/solidus/pull/1 By mimicking the default GitHub format, we make it easier to generate them manually through GitHub's "Generate release notes" [1] button or through the API [2]. At the same time, we're fixing the missing links to the PR when we copy the draft content to the Changelog file, as GitHub doesn't autolink references in files. From their documentation [3]: > Note: Autolinked references are not created in wikis or files in a repository. We'll still miss the link to the contributor, but it's a tradeoff we pay to make our lives easier if the automation fails. Anyway, it can be accessed once landed on the pull request URL. [1] - https://docs.github.com/en/repositories/releasing-projects-on-github/automatically-generated-release-notes [2] - https://docs.github.com/en/rest/releases/releases?apiVersion=2022-11-28#generate-release-notes-content-for-a-release [3] - https://docs.github.com/en/get-started/writing-on-github/working-with-advanced-formatting/autolinked-references-and-url Closes #4904 & #4908 --- dev_tools/lib/solidus/release_drafter.rb | 12 +- .../lib/solidus/release_drafter/builder.rb | 9 +- .../solidus/release_drafter/builder_spec.rb | 129 +++++++++--------- .../spec/lib/solidus/release_drafter_spec.rb | 4 +- 4 files changed, 83 insertions(+), 71 deletions(-) diff --git a/dev_tools/lib/solidus/release_drafter.rb b/dev_tools/lib/solidus/release_drafter.rb index eff53fb9125..a5bb19cb0bc 100644 --- a/dev_tools/lib/solidus/release_drafter.rb +++ b/dev_tools/lib/solidus/release_drafter.rb @@ -37,7 +37,13 @@ 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 + ) .then { |builder| add_pr(builder, pr, matching_labels) } .then { |draft| save_release(draft, candidate_tag, branch) } end @@ -68,5 +74,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..7883dd6a78d 100644 --- a/dev_tools/lib/solidus/release_drafter/builder.rb +++ b/dev_tools/lib/solidus/release_drafter/builder.rb @@ -18,15 +18,16 @@ class Builder TITLE_SEPARATOR = SEPARATOR - ENTRY_TEMPLATE = lambda do |title, number, user| - "- #{title} ##{number} (@#{user})" + ENTRY_TEMPLATE = lambda do |title, link, user| + "* #{title} by @#{user} in #{link}" end.freeze ENTRY_SEPARATOR = SEPARATOR SECTION_SEPARATOR = SEPARATOR + SEPARATOR - def initialize(draft:, categories:, prepend: '', append: '') + def initialize(draft:, categories:, number_link_builder:, prepend: '', append: '') + @number_link_builder = number_link_builder @categories = Set[*categories] @prepend = prepend @prepend_pattern = /^#{Regexp.quote(@prepend)}/m @@ -39,7 +40,7 @@ def initialize(draft:, categories:, prepend: '', append: '') end def add(title:, number:, user:, categories:) - ENTRY_TEMPLATE.(title, number, user) + ENTRY_TEMPLATE.(title, @number_link_builder.(number), user) .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..cd5a1594e40 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,55 @@ 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}" } } 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) + * Old entry 1 by @bob in https://github.com/solidusio/solidus/pull/9 ## 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) }.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) + * Old entry 1 by @bob in https://github.com/solidusio/solidus/pull/9 ## Solidus Backend ## Solidus API - - Old entry 2 #8 (@alice) + * Old entry 2 by @alice in https://github.com/solidusio/solidus/pull/8 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) }.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) + * Old entry 1 by @bob in https://github.com/solidusio/solidus/pull/9 ## Solidus Backend ## Solidus API - - Old entry 2 #8 (@alice) + * Old entry 2 by @alice in https://github.com/solidusio/solidus/pull/8 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) }.to raise_error(/Appended text is not present in the draft/) end end @@ -66,24 +67,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) expect( builder.add(title: 'New entry', number: 10, user: 'alice', categories: [core, api]).content ).to eq <<~TXT.chomp ## Solidus Core - - New entry #10 (@alice) + * New entry by @alice in https://github.com/solidusio/solidus/pull/10 ## Solidus Backend ## Solidus API - - New entry #10 (@alice) + * New entry by @alice in https://github.com/solidusio/solidus/pull/10 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) expect( builder.add(title: 'New entry', number: 10, user: 'alice', categories: ['unknown']).content @@ -100,7 +101,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) expect( builder.add(title: 'New entry', number: 10, user: 'alice', categories: []).content @@ -117,7 +118,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) expect( builder.add(title: 'New entry', number: 10, user: 'alice', categories: [core, api]).content @@ -125,27 +126,27 @@ ## List of changes ## Solidus Core - - New entry #10 (@alice) + * New entry by @alice in https://github.com/solidusio/solidus/pull/10 ## Solidus Backend ## Solidus API - - New entry #10 (@alice) + * New entry by @alice in https://github.com/solidusio/solidus/pull/10 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) expect( builder.add(title: 'New entry', number: 10, user: 'alice', categories: [core, backend]).content ).to eq <<~TXT.chomp ## Solidus Core - - New entry #10 (@alice) + * New entry by @alice in https://github.com/solidusio/solidus/pull/10 ## Solidus Backend - - New entry #10 (@alice) + * New entry by @alice in https://github.com/solidusio/solidus/pull/10 ## Solidus API @@ -159,45 +160,45 @@ it 'adds entry under all matching categories' do content = <<~TXT.chomp ## Solidus Core - - Old entry 1 #9 (@bob) + * Old entry 1 by @bob in https://github.com/solidusio/solidus/pull/9 ## Solidus Backend ## Solidus API - - Old entry 2 #8 (@alice) + * Old entry 2 by @alice in https://github.com/solidusio/solidus/pull/8 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) 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) + * Old entry 1 by @bob in https://github.com/solidusio/solidus/pull/9 + * New entry by @alice in https://github.com/solidusio/solidus/pull/10 ## Solidus Backend - - New entry #10 (@alice) + * New entry by @alice in https://github.com/solidusio/solidus/pull/10 ## Solidus API - - Old entry 2 #8 (@alice) + * Old entry 2 by @alice in https://github.com/solidusio/solidus/pull/8 TXT end it "returns the same when categories don't match" do content = <<~TXT.chomp ## Solidus Core - - Old entry 1 #9 (@bob) + * Old entry 1 by @bob in https://github.com/solidusio/solidus/pull/9 ## Solidus Backend ## Solidus API - - Old entry 2 #8 (@alice) + * Old entry 2 by @alice in https://github.com/solidusio/solidus/pull/8 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) expect( builder.add(title: 'New entry', number: 10, user: 'alice', categories: ['unknown']).content @@ -207,16 +208,16 @@ it "returns the same when categories are empty" do content = <<~TXT.chomp ## Solidus Core - - Old entry 1 #9 (@bob) + * Old entry 1 by @bob in https://github.com/solidusio/solidus/pull/9 ## Solidus Backend ## Solidus API - - Old entry 2 #8 (@alice) + * Old entry 2 by @alice in https://github.com/solidusio/solidus/pull/8 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) expect( builder.add(title: 'New entry', number: 10, user: 'alice', categories: []).content @@ -228,9 +229,9 @@ ## Solidus Core - - Old entry 1 #9 (@bob) + * Old entry 1 by @bob in https://github.com/solidusio/solidus/pull/9 - - Old entry 2 #10 (@alice) + * Old entry 2 by @alice in https://github.com/solidusio/solidus/pull/10 \t ## Solidus Backend @@ -239,18 +240,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) 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) + * Old entry 1 by @bob in https://github.com/solidusio/solidus/pull/9 + * Old entry 2 by @alice in https://github.com/solidusio/solidus/pull/10 + * New entry by @alice in https://github.com/solidusio/solidus/pull/11 ## Solidus Backend - - New entry #11 (@alice) + * New entry by @alice in https://github.com/solidusio/solidus/pull/11 ## Solidus API @@ -261,60 +262,60 @@ content = <<~TXT.chomp Outlier ## Solidus Core - - Old entry 1 #9 (@bob) + * Old entry 1 by @bob in https://github.com/solidusio/solidus/pull/9 ## Solidus Backend ## Solidus API - - Old entry 2 #8 (@alice) + * Old entry 2 by @alice in https://github.com/solidusio/solidus/pull/8 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) 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) + * Old entry 1 by @bob in https://github.com/solidusio/solidus/pull/9 + * New entry by @alice in https://github.com/solidusio/solidus/pull/10 ## Solidus Backend ## Solidus API - - Old entry 2 #8 (@alice) + * Old entry 2 by @alice in https://github.com/solidusio/solidus/pull/8 TXT end it 'keeps unknown content within a category' do content = <<~TXT.chomp ## Solidus Core - - Old entry 1 #9 (@bob) + * Old entry 1 by @bob in https://github.com/solidusio/solidus/pull/9 Outlier ## Solidus Backend ## Solidus API - - Old entry 2 #8 (@alice) + * Old entry 2 by @alice in https://github.com/solidusio/solidus/pull/8 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) expect( builder.add(title: 'New entry', number: 10, user: 'alice', categories: [core]).content ).to eq <<~TXT.chomp ## Solidus Core - - Old entry 1 #9 (@bob) + * Old entry 1 by @bob in https://github.com/solidusio/solidus/pull/9 Outlier - - New entry #10 (@alice) + * New entry by @alice in https://github.com/solidusio/solidus/pull/10 ## Solidus Backend ## Solidus API - - Old entry 2 #8 (@alice) + * Old entry 2 by @alice in https://github.com/solidusio/solidus/pull/8 TXT end @@ -323,15 +324,15 @@ ## List of changes ## Solidus Core - - Old entry 1 #9 (@bob) + * Old entry 1 by @bob in https://github.com/solidusio/solidus/pull/9 ## Solidus Backend ## Solidus API - - Old entry 2 #8 (@alice) + * Old entry 2 by @alice in https://github.com/solidusio/solidus/pull/8 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) expect( builder.add(title: 'New entry', number: 10, user: 'alice', categories: [core, api]).content @@ -339,44 +340,44 @@ ## List of changes ## Solidus Core - - Old entry 1 #9 (@bob) - - New entry #10 (@alice) + * Old entry 1 by @bob in https://github.com/solidusio/solidus/pull/9 + * New entry by @alice in https://github.com/solidusio/solidus/pull/10 ## Solidus Backend ## Solidus API - - Old entry 2 #8 (@alice) - - New entry #10 (@alice) + * Old entry 2 by @alice in https://github.com/solidusio/solidus/pull/8 + * New entry by @alice in https://github.com/solidusio/solidus/pull/10 TXT end it 'keeps appended text in place' do content = <<~TXT.chomp ## Solidus Core - - Old entry 1 #9 (@bob) + * Old entry 1 by @bob in https://github.com/solidusio/solidus/pull/9 ## Solidus Backend ## Solidus API - - Old entry 2 #8 (@alice) + * Old entry 2 by @alice in https://github.com/solidusio/solidus/pull/8 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) expect( builder.add(title: 'New entry', number: 10, user: 'alice', categories: [backend]).content ).to eq <<~TXT.chomp ## Solidus Core - - Old entry 1 #9 (@bob) + * Old entry 1 by @bob in https://github.com/solidusio/solidus/pull/9 ## Solidus Backend - - New entry #10 (@alice) + * New entry by @alice in https://github.com/solidusio/solidus/pull/10 ## Solidus API - - Old entry 2 #8 (@alice) + * Old entry 2 by @alice in https://github.com/solidusio/solidus/pull/8 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..fa8d6a5a9e2 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 by @alice in https://github.com/fake/solidus/pull/1")}/) 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 by @alice in https://github.com/fake/solidus/pull/1")}.*#{Regexp.escape("## Solidus API\n* PR number 1 by @alice in https://github.com/fake/solidus/pull/1")}/m) end it 'ignores if no matching label is present' do