diff --git a/README.md b/README.md
index a878d60..477d94e 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], repo_link: String, org_name: String, modularization_library: String ).returns(String) }
+ def format_offenses(offenses, repo_link, org_name, modularization_library)
# 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..c11ecd0 100644
--- a/lib/danger-packwerk/check/default_formatter.rb
+++ b/lib/danger-packwerk/check/default_formatter.rb
@@ -21,10 +21,11 @@ def initialize(custom_help_message: nil)
override.params(
offenses: T::Array[Packwerk::ReferenceOffense],
repo_link: String,
- org_name: String
+ org_name: String,
+ modularization_library: String
).returns(String)
end
- def format_offenses(offenses, repo_link, org_name)
+ def format_offenses(offenses, repo_link, org_name, modularization_library: 'packwerk')
reference_offense = T.must(offenses.first)
violation_types = offenses.map(&:violation_type)
referencing_file = reference_offense.reference.relative_path
@@ -38,7 +39,11 @@ def format_offenses(offenses, repo_link, org_name)
constant_source_package = T.must(ParsePackwerk.all.find { |p| p.name == constant_source_package_name })
constant_source_package_ownership_info = Private::OwnershipInformation.for_package(constant_source_package, org_name)
- disclaimer = 'Before you run `bin/packwerk update-todo`, check out these quick suggestions:'
+ if modularization_library == 'packwerk'
+ disclaimer = 'Before you run `bin/packwerk update-todo`, check out these quick suggestions:'
+ elsif modularization_library == 'pks'
+ disclaimer = 'Before you run `bin/pks update`, check out these quick suggestions:'
+ end
referencing_code_in_right_pack = "- Does the code you are writing live in the right pack?\n - If not, try `bin/packs move packs/destination_pack #{referencing_file}`"
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}?"
diff --git a/lib/danger-packwerk/check/offenses_formatter.rb b/lib/danger-packwerk/check/offenses_formatter.rb
index 2cb5893..5b3a539 100644
--- a/lib/danger-packwerk/check/offenses_formatter.rb
+++ b/lib/danger-packwerk/check/offenses_formatter.rb
@@ -14,10 +14,11 @@ module OffensesFormatter
abstract.params(
offenses: T::Array[Packwerk::ReferenceOffense],
repo_link: String,
- org_name: String
+ org_name: String,
+ modularization_library: String
).returns(String)
end
- def format_offenses(offenses, repo_link, org_name); end
+ def format_offenses(offenses, repo_link, org_name, modularization_library: 'packwerk'); 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..4511618 100644
--- a/lib/danger-packwerk/danger_package_todo_yml_changes.rb
+++ b/lib/danger-packwerk/danger_package_todo_yml_changes.rb
@@ -30,7 +30,8 @@ class DangerPackageTodoYmlChanges < Danger::Plugin
before_comment: BeforeComment,
max_comments: Integer,
violation_types: T::Array[String],
- root_path: T.nilable(String)
+ root_path: T.nilable(String),
+ modularization_library: String
).void
end
def check(
@@ -38,7 +39,8 @@ def check(
before_comment: DEFAULT_BEFORE_COMMENT,
max_comments: DEFAULT_MAX_COMMENTS,
violation_types: DEFAULT_VIOLATION_TYPES,
- root_path: nil
+ root_path: nil,
+ modularization_library: 'packwerk'
)
offenses_formatter ||= Update::DefaultFormatter.new
repo_link = github.pr_json[:base][:repo][:html_url]
@@ -62,7 +64,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, repo_link, org_name, modularization_library: modularization_library),
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..9e3aeaf 100644
--- a/lib/danger-packwerk/danger_packwerk.rb
+++ b/lib/danger-packwerk/danger_packwerk.rb
@@ -45,7 +45,8 @@ class CommentGroupingStrategy < ::T::Enum
on_failure: OnFailure,
violation_types: T::Array[String],
grouping_strategy: CommentGroupingStrategy,
- root_path: T.nilable(String)
+ root_path: T.nilable(String),
+ modularization_library: String
).void
end
def check(
@@ -56,7 +57,8 @@ def check(
on_failure: DEFAULT_ON_FAILURE,
violation_types: DEFAULT_VIOLATION_TYPES,
grouping_strategy: CommentGroupingStrategy::PerConstantPerLocation,
- root_path: nil
+ root_path: nil,
+ modularization_library: 'packwerk'
)
offenses_formatter ||= Check::DefaultFormatter.new
repo_link = github.pr_json[:base][:repo][:html_url]
@@ -136,7 +138,12 @@ 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,
+ repo_link,
+ org_name,
+ modularization_library: modularization_library
+ )
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..58ffc5c 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], repo_link: String, org_name: String, modularization_library: String).returns(String) }
+ def format_offenses(offenses, repo_link, org_name, modularization_library: 'packwerk')
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.
@@ -28,7 +28,11 @@ def format_offenses(offenses, repo_link, org_name)
package_referring_to_constant_owner = Private::OwnershipInformation.for_package(referencing_file_pack, org_name)
- disclaimer = 'We noticed you ran `bin/packwerk update-todo`. Check out [the docs](https://github.com/Shopify/packwerk/blob/main/RESOLVING_VIOLATIONS.md) to see other ways to resolve violations.'
+ if modularization_library == 'packwerk'
+ disclaimer = 'We noticed you ran `bin/packwerk update-todo`. Check out [the docs](https://github.com/Shopify/packwerk/blob/main/RESOLVING_VIOLATIONS.md) to see other ways to resolve violations.'
+ elsif modularization_library == 'pks'
+ disclaimer = 'We noticed you ran `bin/pks update`. Check out [the docs](https://github.com/Shopify/packwerk/blob/main/RESOLVING_VIOLATIONS.md) to see other ways to resolve violations.'
+ end
pluralized_violation = offenses.count > 1 ? 'these violations' : 'this violation'
request_to_add_context = "- Could you add some context as a reply here about why we needed to add #{pluralized_violation}?\n"
diff --git a/lib/danger-packwerk/update/offenses_formatter.rb b/lib/danger-packwerk/update/offenses_formatter.rb
index 326bd98..36f53fc 100644
--- a/lib/danger-packwerk/update/offenses_formatter.rb
+++ b/lib/danger-packwerk/update/offenses_formatter.rb
@@ -14,10 +14,11 @@ module OffensesFormatter
abstract.params(
offenses: T::Array[BasicReferenceOffense],
repo_link: String,
- org_name: String
+ org_name: String,
+ modularization_library: String
).returns(String)
end
- def format_offenses(offenses, repo_link, org_name); end
+ def format_offenses(offenses, repo_link, org_name, modularization_library: 'packwerk'); 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..4c05db4 100644
--- a/spec/danger_packwerk/danger_package_todo_yml_changes_spec.rb
+++ b/spec/danger_packwerk/danger_package_todo_yml_changes_spec.rb
@@ -29,10 +29,12 @@ module DangerPackwerk
subject do
danger_package_todo_yml_changes.check(
before_comment: before_comment,
- root_path: root_path
+ root_path: root_path,
+ modularization_library: modularization_library
)
end
+ let(:modularization_library) { 'packwerk' }
let(:some_pack_package_todo_before) { nil }
let(:diff_double) { sorbet_double(Git::Diff::DiffFile) }
@@ -139,6 +141,34 @@ module DangerPackwerk
EXPECTED
).and_nothing_else
end
+
+ context 'a modularization_library is passed in' do
+ let!(:modularization_library) { 'pks' }
+
+ it 'displays a markdown with a reasonable message' do
+ subject
+
+ expect('packs/some_pack/package_todo.yml').to contain_inline_markdown(
+ <<~EXPECTED
+ ---
+ packs/some_other_pack:
+ "OtherPackClass":
+ violations:
+ - privacy
+ - dependency
+ files:
+ - packs/some_pack/some_class.rb
+ ==================== DANGER_START
+ Hi again! It looks like `OtherPackClass` is private API of `packs/some_other_pack`, which is also not in `packs/some_pack`'s list of dependencies.
+ We noticed you ran `bin/pks update`. Check out [the docs](https://github.com/Shopify/packwerk/blob/main/RESOLVING_VIOLATIONS.md) to see other ways to resolve violations.
+
+ - Could you add some context as a reply here about why we needed to add these violations?
+
+ ==================== DANGER_END
+ EXPECTED
+ ).and_nothing_else
+ end
+ end
end
context 'a offenses formatter is passed in' do
@@ -146,7 +176,7 @@ module DangerPackwerk
Class.new do
include Update::OffensesFormatter
- def format_offenses(added_violations, repo_link, org_name)
+ def format_offenses(added_violations, repo_link, org_name, modularization_library: 'packwerk')
<<~MESSAGE
There are #{added_violations.count} new violations,
with class_names #{added_violations.map(&:class_name).uniq.sort},
@@ -1160,7 +1190,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, repo_link, org_name, modularization_library: 'packwerk')
<<~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..6e98b55 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, repo_link, org_name, modularization_library:)
offenses.map(&:message).join("\n\n")
end
end
@@ -775,7 +775,8 @@ def format_offenses(offenses, repo_link, org_name)
grouping_strategy: DangerPackwerk::PerConstantPerPackGrouping,
offenses_formatter: Check::DefaultFormatter.new(
custom_help_message: 'Need help? Join us in #ruby-modularity or see go/packs.'
- )
+ ),
+ modularization_library: modularization_library
)
end
@@ -816,6 +817,7 @@ def format_offenses(offenses, repo_link, org_name)
YML
end
+ let(:modularization_library) { 'packwerk' }
let(:danger_packwerk) { dangerfile.packwerk }
let(:generic_privacy_violation) do
sorbet_double(
@@ -889,6 +891,46 @@ def format_offenses(offenses, repo_link, org_name)
expect(actual_markdown.type).to eq :markdown
end
+ context 'modularization_library is pks' do
+ let(:modularization_library) { 'pks' }
+
+ it 'uses the pks update command' do
+ subject
+ expect(dangerfile.status_report[:warnings]).to be_empty
+ expect(dangerfile.status_report[:errors]).to be_empty
+ actual_markdowns = dangerfile.status_report[:markdowns]
+ expect(actual_markdowns.count).to eq 1
+ actual_markdown = actual_markdowns.first
+ 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)
+ - 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))
+
+ Quick suggestions :bulb:
+
+ Before you run `bin/pks update`, check out these quick suggestions:
+ - Does the code you are writing live in the right pack?
+ - If not, try `bin/packs move packs/destination_pack packs/referencing_package/some_file.rb`
+ - Does PrivateConstant live in the right pack?
+ - If not, try `bin/packs move packs/destination_pack packs/gusto_slack/app/services/private_constant.rb`
+ - Does API in packs/gusto_slack/public support this use case?
+ - If not, can we work with [@MyOrg/product-infrastructure](https://github.com/orgs/MyOrg/teams/product-infrastructure/members) to create and use a public API?
+ - If `PrivateConstant` should already be public, try `bin/packs make_public packs/gusto_slack/app/services/private_constant.rb`.
+
+