diff --git a/README.md b/README.md index a878d60..99b2074 100644 --- a/README.md +++ b/README.md @@ -43,8 +43,8 @@ class MyFormatter extend T::Sig include DangerPackwerk::Check::OffensesFormatter # Packwerk::ReferenceOffense: https://github.com/Shopify/packwerk/blob/main/lib/packwerk/reference_offense.rb - sig { override.params(offenses: T::Array[Packwerk::ReferenceOffense], repo_link: String, org_name: String).returns(String) } - def format_offenses(offenses, repo_link, org_name) + sig { override.params(offenses: T::Array[Packwerk::ReferenceOffense], plugin: Danger::Plugin, org_name: String).returns(String) } + def format_offenses(offenses, plugin, org_name) # your logic here end end @@ -98,8 +98,8 @@ class MyFormatter extend T::Sig include DangerPackwerk::Update::OffensesFormatter # DangerPackwerk::BasicReferenceOffense - sig { override.params(offenses: T::Array[DangerPackwerk::BasicReferenceOffense], repo_link: String, org_name: String).returns(String) } - def format_offenses(offenses, repo_link, org_name) + sig { override.params(offenses: T::Array[DangerPackwerk::BasicReferenceOffense], plugin: Danger::Plugin, org_name: String).returns(String) } + def format_offenses(offenses, plugin, org_name) # your logic here end end diff --git a/lib/danger-packwerk/check/default_formatter.rb b/lib/danger-packwerk/check/default_formatter.rb index e00308f..c60d5b3 100644 --- a/lib/danger-packwerk/check/default_formatter.rb +++ b/lib/danger-packwerk/check/default_formatter.rb @@ -20,11 +20,11 @@ def initialize(custom_help_message: nil) sig do override.params( offenses: T::Array[Packwerk::ReferenceOffense], - repo_link: String, + plugin: Danger::Plugin, org_name: String ).returns(String) end - def format_offenses(offenses, repo_link, org_name) + def format_offenses(offenses, plugin, org_name) reference_offense = T.must(offenses.first) violation_types = offenses.map(&:violation_type) referencing_file = reference_offense.reference.relative_path @@ -43,13 +43,15 @@ def format_offenses(offenses, repo_link, org_name) referenced_code_in_right_pack = "- Does #{constant_name} live in the right pack?\n - If not, try `bin/packs move packs/destination_pack #{constant_location}`" dependency_violation_message = "- Do we actually want to depend on #{constant_source_package_name}?\n - If so, try `bin/packs add_dependency #{referencing_file_pack} #{constant_source_package_name}`\n - If not, what can we change about the design so we do not have to depend on #{constant_source_package_name}?" team_to_work_with = constant_source_package_ownership_info.owning_team ? constant_source_package_ownership_info.markdown_link_to_github_members_no_tag : 'the pack owner' + privacy_violation_message = "- Does API in #{constant_source_package.name}/public support this use case?\n - If not, can we work with #{team_to_work_with} to create and use a public API?\n - If `#{constant_name}` should already be public, try `bin/packs make_public #{constant_location}`." + constant_link = "`#{constant_name}` (#{plugin.github.html_link(constant_location)})" if violation_types.include?(::DangerPackwerk::DEPENDENCY_VIOLATION_TYPE) && violation_types.include?(::DangerPackwerk::PRIVACY_VIOLATION_TYPE) <<~MESSAGE **Packwerk Violation** - Type: Privacy :lock: + Dependency :knot: - - Constant: [`#{constant_name}`](#{repo_link}/blob/main/#{constant_location}) + - Constant: #{constant_link} - Owning pack: #{constant_source_package_name} #{constant_source_package_ownership_info.ownership_copy} @@ -69,7 +71,7 @@ def format_offenses(offenses, repo_link, org_name) <<~MESSAGE **Packwerk Violation** - Type: Dependency :knot: - - Constant: [`#{constant_name}`](#{repo_link}/blob/main/#{constant_location}) + - Constant: #{constant_link} - Owning pack: #{constant_source_package_name} #{constant_source_package_ownership_info.ownership_copy} @@ -88,7 +90,7 @@ def format_offenses(offenses, repo_link, org_name) <<~MESSAGE **Packwerk Violation** - Type: Privacy :lock: - - Constant: [`#{constant_name}`](#{repo_link}/blob/main/#{constant_location}) + - Constant: #{constant_link} - Owning pack: #{constant_source_package_name} #{constant_source_package_ownership_info.ownership_copy} diff --git a/lib/danger-packwerk/check/offenses_formatter.rb b/lib/danger-packwerk/check/offenses_formatter.rb index 2cb5893..e306d61 100644 --- a/lib/danger-packwerk/check/offenses_formatter.rb +++ b/lib/danger-packwerk/check/offenses_formatter.rb @@ -13,11 +13,11 @@ module OffensesFormatter sig do abstract.params( offenses: T::Array[Packwerk::ReferenceOffense], - repo_link: String, + plugin: Danger::Plugin, org_name: String ).returns(String) end - def format_offenses(offenses, repo_link, org_name); end + def format_offenses(offenses, plugin, org_name); end end end end diff --git a/lib/danger-packwerk/danger_package_todo_yml_changes.rb b/lib/danger-packwerk/danger_package_todo_yml_changes.rb index b44ad74..76798bc 100644 --- a/lib/danger-packwerk/danger_package_todo_yml_changes.rb +++ b/lib/danger-packwerk/danger_package_todo_yml_changes.rb @@ -41,7 +41,6 @@ def check( root_path: nil ) offenses_formatter ||= Update::DefaultFormatter.new - repo_link = github.pr_json[:base][:repo][:html_url] org_name = github.pr_json[:base][:repo][:owner][:login] git_filesystem = Private::GitFilesystem.new(git: git, root: root_path || '') @@ -62,7 +61,7 @@ def check( location = T.must(violations.first).file_location markdown( - offenses_formatter.format_offenses(violations, repo_link, org_name), + offenses_formatter.format_offenses(violations, self, org_name), line: location.line_number, file: git_filesystem.convert_to_filesystem(location.file) ) diff --git a/lib/danger-packwerk/danger_packwerk.rb b/lib/danger-packwerk/danger_packwerk.rb index 8ab97df..b45918c 100644 --- a/lib/danger-packwerk/danger_packwerk.rb +++ b/lib/danger-packwerk/danger_packwerk.rb @@ -59,7 +59,6 @@ def check( root_path: nil ) offenses_formatter ||= Check::DefaultFormatter.new - repo_link = github.pr_json[:base][:repo][:html_url] org_name = github.pr_json[:base][:repo][:owner][:login] # This is important because by default, Danger will leave a concantenated list of all its messages if it can't find a commentable place in the @@ -136,7 +135,7 @@ def check( line_number = reference_offense.location&.line referencing_file = reference_offense.reference.relative_path - message = offenses_formatter.format_offenses(unique_packwerk_reference_offenses, repo_link, org_name) + message = offenses_formatter.format_offenses(unique_packwerk_reference_offenses, self, org_name) markdown(message, file: git_filesystem.convert_to_filesystem(referencing_file), line: line_number) end diff --git a/lib/danger-packwerk/update/default_formatter.rb b/lib/danger-packwerk/update/default_formatter.rb index b557dcf..3254700 100644 --- a/lib/danger-packwerk/update/default_formatter.rb +++ b/lib/danger-packwerk/update/default_formatter.rb @@ -15,8 +15,8 @@ def initialize(custom_help_message: nil) @custom_help_message = custom_help_message end - sig { override.params(offenses: T::Array[BasicReferenceOffense], repo_link: String, org_name: String).returns(String) } - def format_offenses(offenses, repo_link, org_name) + sig { override.params(offenses: T::Array[BasicReferenceOffense], plugin: Danger::Plugin, org_name: String).returns(String) } + def format_offenses(offenses, plugin, org_name) violation = T.must(offenses.first) referencing_file_pack = ParsePackwerk.package_from_path(violation.file) # We remove leading double colons as they feel like an implementation detail of packwerk. diff --git a/lib/danger-packwerk/update/offenses_formatter.rb b/lib/danger-packwerk/update/offenses_formatter.rb index 326bd98..9f94c99 100644 --- a/lib/danger-packwerk/update/offenses_formatter.rb +++ b/lib/danger-packwerk/update/offenses_formatter.rb @@ -13,11 +13,11 @@ module OffensesFormatter sig do abstract.params( offenses: T::Array[BasicReferenceOffense], - repo_link: String, + plugin: Danger::Plugin, org_name: String ).returns(String) end - def format_offenses(offenses, repo_link, org_name); end + def format_offenses(offenses, plugin, org_name); end end end end diff --git a/spec/danger_packwerk/danger_package_todo_yml_changes_spec.rb b/spec/danger_packwerk/danger_package_todo_yml_changes_spec.rb index ec958da..fafcea4 100644 --- a/spec/danger_packwerk/danger_package_todo_yml_changes_spec.rb +++ b/spec/danger_packwerk/danger_package_todo_yml_changes_spec.rb @@ -146,7 +146,7 @@ module DangerPackwerk Class.new do include Update::OffensesFormatter - def format_offenses(added_violations, repo_link, org_name) + def format_offenses(added_violations, plugin, org_name) <<~MESSAGE There are #{added_violations.count} new violations, with class_names #{added_violations.map(&:class_name).uniq.sort}, @@ -1160,7 +1160,7 @@ def format_offenses(added_violations, repo_link, org_name) Class.new do include Update::OffensesFormatter - def format_offenses(added_violations, repo_link, org_name) + def format_offenses(added_violations, plugin, org_name) <<~MESSAGE There are #{added_violations.count} new violations, with class_names #{added_violations.map(&:class_name).uniq.sort}, diff --git a/spec/danger_packwerk/danger_packwerk_spec.rb b/spec/danger_packwerk/danger_packwerk_spec.rb index 0e53255..93f7ac7 100644 --- a/spec/danger_packwerk/danger_packwerk_spec.rb +++ b/spec/danger_packwerk/danger_packwerk_spec.rb @@ -30,7 +30,7 @@ module DangerPackwerk Class.new do include Check::OffensesFormatter - def format_offenses(offenses, repo_link, org_name) + def format_offenses(offenses, plugin, org_name) offenses.map(&:message).join("\n\n") end end @@ -863,7 +863,7 @@ def format_offenses(offenses, repo_link, org_name) expected = <<~EXPECTED **Packwerk Violation** - Type: Privacy :lock: - - Constant: [`PrivateConstant`](https://github.com/MyOrg/my_repo/blob/main/packs/gusto_slack/app/services/private_constant.rb) + - Constant: `PrivateConstant` (packs/gusto_slack/app/services/private_constant.rb) - Owning pack: packs/gusto_slack - Owned by [@MyOrg/product-infrastructure](https://github.com/orgs/MyOrg/teams/product-infrastructure/members) (Slack: [#prod-infra](https://slack.com/app_redirect?channel=prod-infra)) @@ -904,7 +904,7 @@ def format_offenses(offenses, repo_link, org_name) expected = <<~EXPECTED **Packwerk Violation** - Type: Privacy :lock: - - Constant: [`PrivateConstant`](https://github.com/MyOrg/my_repo/blob/main/packs/gusto_slack/app/services/private_constant.rb) + - Constant: `PrivateConstant` (packs/gusto_slack/app/services/private_constant.rb) - Owning pack: packs/gusto_slack - This pack is unowned. @@ -947,7 +947,7 @@ def format_offenses(offenses, repo_link, org_name) expected = <<~EXPECTED **Packwerk Violation** - Type: Dependency :knot: - - Constant: [`PrivateConstant`](https://github.com/MyOrg/my_repo/blob/main/packs/gusto_slack/app/services/private_constant.rb) + - Constant: `PrivateConstant` (packs/gusto_slack/app/services/private_constant.rb) - Owning pack: packs/gusto_slack - Owned by [@MyOrg/product-infrastructure](https://github.com/orgs/MyOrg/teams/product-infrastructure/members) (Slack: [#prod-infra](https://slack.com/app_redirect?channel=prod-infra)) @@ -988,7 +988,7 @@ def format_offenses(offenses, repo_link, org_name) expected = <<~EXPECTED **Packwerk Violation** - Type: Privacy :lock: + Dependency :knot: - - Constant: [`PrivateConstant`](https://github.com/MyOrg/my_repo/blob/main/packs/gusto_slack/app/services/private_constant.rb) + - Constant: `PrivateConstant` (packs/gusto_slack/app/services/private_constant.rb) - Owning pack: packs/gusto_slack - Owned by [@MyOrg/product-infrastructure](https://github.com/orgs/MyOrg/teams/product-infrastructure/members) (Slack: [#prod-infra](https://slack.com/app_redirect?channel=prod-infra)) @@ -1061,7 +1061,7 @@ def format_offenses(offenses, repo_link, org_name) expected = <<~EXPECTED **Packwerk Violation** - Type: Privacy :lock: - - Constant: [`PrivateConstant`](https://github.com/MyOrg/my_repo/blob/main/packs/gusto_slack/app/services/private_constant.rb) + - Constant: `PrivateConstant` (packs/gusto_slack/app/services/private_constant.rb) - Owning pack: packs/gusto_slack - Owned by [@MyOrg/product-infrastructure](https://github.com/orgs/MyOrg/teams/product-infrastructure/members) (Slack: [#prod-infra](https://slack.com/app_redirect?channel=prod-infra)) diff --git a/spec/support/danger_plugin.rb b/spec/support/danger_plugin.rb index a5d0fad..7b9bc5b 100644 --- a/spec/support/danger_plugin.rb +++ b/spec/support/danger_plugin.rb @@ -24,5 +24,9 @@ allow(plugin).to receive(:git).and_return(mock_git) allow(plugin.github).to receive(:pr_json).and_return(pr_json) + allow(plugin.github).to receive(:html_link) do |location| + href = "#{pr_json[:base][:repo][:html_url]}/blob/main/#{location}" + "#{location}" + end end end