Skip to content
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

Rename the Piwik\Plugin\Manager class #7656

Open
mnapoli opened this issue Apr 10, 2015 · 2 comments
Open

Rename the Piwik\Plugin\Manager class #7656

mnapoli opened this issue Apr 10, 2015 · 2 comments
Labels
RFC Indicates the issue is a request for comments where the author is looking for feedback. Task Indicates an issue is neither a feature nor a bug and it's purely a "technical" change.

Comments

@mnapoli
Copy link
Contributor

mnapoli commented Apr 10, 2015

Following #7644 the plugin manager will be stored in the DI container, which means we will be able to inject it by simply type-hinting it:

class MyService {
    public function __construct(Manager $pluginManager) {
        // ...
    }
}

Manager alone is confusing (the name doesn't mean anything) and can lead to conflicts. This is already a problem (we end up e.g. aliasing the class name in a lot of places) but I think it's a good time to take the opportunity to rename it as the problem will only get more visible when using dependency injection.

It could be renamed to Piwik\Plugin\PluginManager for example.

Of course backward compatibility would be kept and Piwik\Plugin\Manager still exist as an alias to the new class

Thoughts?

@mnapoli mnapoli added Task Indicates an issue is neither a feature nor a bug and it's purely a "technical" change. RFC Indicates the issue is a request for comments where the author is looking for feedback. labels Apr 10, 2015
@mattab
Copy link
Member

mattab commented Apr 10, 2015

It could be renamed to Piwik\Plugin\PluginManager for example.

+1

@mnapoli
Copy link
Contributor Author

mnapoli commented May 21, 2015

After thinking about it I think it's best to do that for 3.0 as there will be less risks of conflicts with the DI refactorings and we probably won't have the same level of BC to keep.

@mnapoli mnapoli added this to the 3.0.0 milestone May 21, 2015
@mattab mattab modified the milestones: Mid term, 3.0.0 Aug 13, 2015
@mattab mattab modified the milestones: Long term, Mid term Dec 5, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
RFC Indicates the issue is a request for comments where the author is looking for feedback. Task Indicates an issue is neither a feature nor a bug and it's purely a "technical" change.
Projects
None yet
Development

No branches or pull requests

2 participants