-
-
Notifications
You must be signed in to change notification settings - Fork 47
added add-shouldOnlyDependOnComponents #278
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Codecov Report
@@ Coverage Diff @@
## main #278 +/- ##
============================================
+ Coverage 93.55% 93.62% +0.07%
- Complexity 367 370 +3
============================================
Files 57 57
Lines 977 988 +11
============================================
+ Hits 914 925 +11
Misses 63 63
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
| /** @var array<string, string[]> */ | ||
| private $allowedDependencies; | ||
| /** @var array<string, string[]> */ | ||
| private $allowedSpecificDependencies; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cosa cambia tra le allowedDependencies e le allowedSpecificDependencies?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
allowedDependencies is used for creating the rule NotDependsOnTheseNamespaces.
allowedSpecificDependencies is used for creating the rule DependsOnlyOnTheseNamespaces.
I don't like the design now but I would like to iterate on it later.
What do you think @pfazzi ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I think the implementation is a bit misleading ATM. We should iterate over it, but it could lead to a breaking change.
Anyway in this step I would rename this variable to something more self-explaining, like componentDependsOnlyOnTheseNamespaces.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with you @pfazzi.
I renamed the variable with your suggestion :)
In this PR I added the possibility to filter components with
shouldOnlyDependOnComponentsfunction that generates a rule:DependsOnlyOnTheseNamespacesThis can fix some problems about issue #277