-
-
Notifications
You must be signed in to change notification settings - Fork 57
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
Skip namespaces #36
base: master
Are you sure you want to change the base?
Skip namespaces #36
Conversation
Tests are not passing, so will fix the problem and open a new PR |
Fixed |
@leoloso As discussed on Twitter earlier today, sorry for not getting back to you sooner. Life sometimes just gets in the way. You correctly pointed out that I should have been more communicative about it and not leave you in the dark like this. There is no point from my side to continue discussing this over and over again, I dropped the ball and I promise to do better in the future. Let's get back to what this PR is about and why it hasn't been merged yet. I like this new feature, but not the way it has been implemented. This basically changes the method signature of a lot of methods, where the parameter isn't used. It comes down to passing a variable through all the replacing methods, which is rarely used. I appreciate the effort to make the code more readable, but should we ever want to use another parameter like this, it just keeps on changing the method signature for each individual parameter. Therefore I propose that I make a Config object that holds the Mozart configuration as read from the project |
I have no objections to your proposed changes :) All I care about is the functionality, not how it is implemented. So please go ahead and reject this PR. My use case is this one: my project has a few dozens components under namespace "PoP" (PoP\Engine, PoP\Root, etc) and many others from everyone else (Symfony, etc). All the "PoP" ones need not be scoped, so I want to add a rule that says: skip everything starting with "PoP" |
Instead of a config object, why not simply use a DI container as a single source of truth for the whole application? |
Fix for #27