From 8fa050016847c30eeeb36f028e171d084e545e88 Mon Sep 17 00:00:00 2001 From: Derek Prior Date: Fri, 17 Jan 2025 16:57:20 -0500 Subject: [PATCH] Clarify dependency violation message I was bothered by a couple of things in the dependency violation message and this change fixes them. 1. "Are we missing an abstraction?" is not helpful context. 2. "Is the code making the reference, and the referenced constant, in the right packages?" was not grammatically correct or clear. --- TROUBLESHOOT.md | 3 +-- lib/packwerk/reference_checking/checkers/dependency_checker.rb | 3 +-- test/unit/packwerk/reference_offense_test.rb | 3 +-- 3 files changed, 3 insertions(+), 6 deletions(-) diff --git a/TROUBLESHOOT.md b/TROUBLESHOOT.md index 5e66f0ca9..d64511378 100644 --- a/TROUBLESHOOT.md +++ b/TROUBLESHOOT.md @@ -28,8 +28,7 @@ See: [USAGE.md - Enforcing dependency boundary](USAGE.md#Enforcing-dependency-bo > /Users/JaneDoe/src/github.com/sample-project/billing/app/jobs/document_processing_job.rb:48:6 > Dependency violation: ::Edi::Source belongs to 'edi', but 'billing' does not specify a dependency on 'edi'. -> Are we missing an abstraction? -> Is the code making the reference, and the referenced constant, in the right packages? +> Are the constant and its references in the right packages? > > Inference details: 'Edi::Source' refers to ::Edi::Source which seems to be defined in edi/app/models/edi/source.rb. diff --git a/lib/packwerk/reference_checking/checkers/dependency_checker.rb b/lib/packwerk/reference_checking/checkers/dependency_checker.rb index 47f901a44..284439911 100644 --- a/lib/packwerk/reference_checking/checkers/dependency_checker.rb +++ b/lib/packwerk/reference_checking/checkers/dependency_checker.rb @@ -40,8 +40,7 @@ def message(reference) <<~EOS Dependency violation: #{const_name} belongs to '#{const_package}', but '#{ref_package}' does not specify a dependency on '#{const_package}'. - Are we missing an abstraction? - Is the code making the reference, and the referenced constant, in the right packages? + Are the constant and its references in the right packages? #{standard_help_message(reference)} EOS diff --git a/test/unit/packwerk/reference_offense_test.rb b/test/unit/packwerk/reference_offense_test.rb index 65e9e80a5..224e31122 100644 --- a/test/unit/packwerk/reference_offense_test.rb +++ b/test/unit/packwerk/reference_offense_test.rb @@ -29,8 +29,7 @@ class ReferenceOffenseTest < Minitest::Test expected = <<~EXPECTED Dependency violation: ::SomeName belongs to 'destination_package', but 'components/source' does not specify a dependency on 'destination_package'. - Are we missing an abstraction? - Is the code making the reference, and the referenced constant, in the right packages? + Are the constant and its references in the right packages? Inference details: this is a reference to ::SomeName which seems to be defined in some/location.rb. To receive help interpreting or resolving this error message, see: https://github.com/Shopify/packwerk/blob/main/TROUBLESHOOT.md#Troubleshooting-violations