From 90357fc9fc6ac8ac312c0aca0dc11a5b6049dadb Mon Sep 17 00:00:00 2001 From: David McIntosh <804610+mctofu@users.noreply.github.com> Date: Wed, 21 Sep 2022 10:05:30 -0700 Subject: [PATCH 1/2] Explain why a dependency was removed in the PR --- .../pull_request_creator/message_builder.rb | 24 +++++++++++++++++-- .../message_builder_spec.rb | 6 ++--- 2 files changed, 25 insertions(+), 5 deletions(-) diff --git a/common/lib/dependabot/pull_request_creator/message_builder.rb b/common/lib/dependabot/pull_request_creator/message_builder.rb index ce23bffbddd..df46572a78b 100644 --- a/common/lib/dependabot/pull_request_creator/message_builder.rb +++ b/common/lib/dependabot/pull_request_creator/message_builder.rb @@ -198,9 +198,10 @@ def version_commit_message_intro return dependency_set_intro if dependencies.count > 1 && updating_a_dependency_set? + return transitive_removed_dependency_intro if dependencies.count > 1 && removing_a_transitive_dependency? + return transitive_multidependency_intro if dependencies.count > 1 && - updating_top_level_and_transitive_dependencies? && - dependencies.none?(&:removed?) + updating_top_level_and_transitive_dependencies? return multidependency_intro if dependencies.count > 1 @@ -262,6 +263,21 @@ def transitive_multidependency_intro msg end + def transitive_removed_dependency_intro + msg = "Removes #{dependency_links[0]}. It's no longer used after updating" + + msg += if dependencies.count > 2 + " ancestor dependencies #{dependency_links[0..-2].join(', ')} " \ + "and #{dependency_links[-1]}. " + else + " ancestor dependency #{dependency_links[1]}. " + end + + msg += "These dependencies need to be updated together.\n" + + msg + end + def from_version_msg(previous_version) return "" unless previous_version @@ -280,6 +296,10 @@ def updating_a_dependency_set? any? { |r| r.dig(:metadata, :dependency_set) } end + def removing_a_transitive_dependency? + dependencies.any?(&:removed?) + end + def updating_top_level_and_transitive_dependencies? dependencies.any?(&:top_level?) && dependencies.any? { |dep| !dep.top_level? } diff --git a/common/spec/dependabot/pull_request_creator/message_builder_spec.rb b/common/spec/dependabot/pull_request_creator/message_builder_spec.rb index 4107a72a765..9ed47ef6e69 100644 --- a/common/spec/dependabot/pull_request_creator/message_builder_spec.rb +++ b/common/spec/dependabot/pull_request_creator/message_builder_spec.rb @@ -1612,9 +1612,9 @@ def commits_details(base:, head:) it "includes details of both dependencies" do expect(pr_message). to eq( - "Bumps [statesman](https://github.com/gocardless/statesman) " \ - "and [business](https://github.com/gocardless/business). " \ - "These dependencies needed to be updated together.\n" \ + "Removes [statesman](https://github.com/gocardless/statesman). It's no longer used after updating " \ + "ancestor dependency [business](https://github.com/gocardless/business). " \ + "These dependencies need to be updated together.\n\n" \ "Removes `statesman`\n" \ "Updates `business` from 1.4.0 to 1.5.0\n" \ "
\n" \ From 1151c79ffb481c001adfd3e3b1aae14e988bd54b Mon Sep 17 00:00:00 2001 From: David McIntosh <804610+mctofu@users.noreply.github.com> Date: Wed, 21 Sep 2022 10:14:39 -0700 Subject: [PATCH 2/2] Include newline between removed & updated dependency --- common/lib/dependabot/pull_request_creator/message_builder.rb | 2 +- .../dependabot/pull_request_creator/message_builder_spec.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/common/lib/dependabot/pull_request_creator/message_builder.rb b/common/lib/dependabot/pull_request_creator/message_builder.rb index df46572a78b..64060ac3819 100644 --- a/common/lib/dependabot/pull_request_creator/message_builder.rb +++ b/common/lib/dependabot/pull_request_creator/message_builder.rb @@ -366,7 +366,7 @@ def metadata_cascades dependencies.map do |dep| msg = if dep.removed? - "\nRemoves `#{dep.display_name}`" + "\nRemoves `#{dep.display_name}`\n" else "\nUpdates `#{dep.display_name}` " \ "#{from_version_msg(previous_version(dep))}" \ diff --git a/common/spec/dependabot/pull_request_creator/message_builder_spec.rb b/common/spec/dependabot/pull_request_creator/message_builder_spec.rb index 9ed47ef6e69..9757d02d5df 100644 --- a/common/spec/dependabot/pull_request_creator/message_builder_spec.rb +++ b/common/spec/dependabot/pull_request_creator/message_builder_spec.rb @@ -1615,7 +1615,7 @@ def commits_details(base:, head:) "Removes [statesman](https://github.com/gocardless/statesman). It's no longer used after updating " \ "ancestor dependency [business](https://github.com/gocardless/business). " \ "These dependencies need to be updated together.\n\n" \ - "Removes `statesman`\n" \ + "Removes `statesman`\n\n" \ "Updates `business` from 1.4.0 to 1.5.0\n" \ "
\n" \ "Changelog\n" \