mozart icon indicating copy to clipboard operation
mozart copied to clipboard

Skip namespaces

Open leoloso opened this issue 4 years ago • 5 comments

Fix for https://github.com/coenjacobs/mozart/issues/27

leoloso avatar Feb 06 '20 07:02 leoloso

Tests are not passing, so will fix the problem and open a new PR

leoloso avatar Feb 06 '20 07:02 leoloso

Fixed

leoloso avatar Feb 06 '20 07:02 leoloso

@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 composer.json file. That Config object can then be passed on to the various classes having a need for it and your logic can then use that Config object to check if your specific parameter is being used. Does that sound like a good idea, also bearing in mind that this will be used for the core configuration and more optional parameters (for example the newly introduced delete_vendor_directories and override_autoload, which are using their own unique approach as well)? I will write up a proposal for this Config object this weekend.

coenjacobs avatar May 23 '20 09:05 coenjacobs

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"

leoloso avatar May 23 '20 09:05 leoloso

Instead of a config object, why not simply use a DI container as a single source of truth for the whole application?

XedinUnknown avatar Dec 01 '20 13:12 XedinUnknown