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

Add plugins in composer autoload file list #23

Merged
merged 2 commits into from
Jul 12, 2018

Conversation

ricardotulio
Copy link
Contributor

What this PR do?

Force to declare plugin classes adding source files into file list of autoload section in composer.json.

What this PR solve?

The plugins are loaded dynamically using get_declared_class function. The problem is that these class will never be until be used.

This commit just add this files in composer autoload files to be declared when autoload.php be included.

This problem was referenced in #20

The plugins are loaded dinamically using get_declared_class function.
The problem is that these class will never be declared if its never
loaded.

This commit this add this files in composer autoload files to be
declared when autoload.php be included.
@ricardotulio ricardotulio changed the title plugins: add plugins in composer autoload file list Add plugins in composer autoload file list May 22, 2018
@marciol
Copy link

marciol commented May 23, 2018

I think that this approach leads to a more explicit code, allowing us to reason more directly about the dependencies, at least it is what I learned from the past experiences in software development. Less magic leads to more maintainability.

@renatocassino
Copy link
Owner

Hello @ricardotulio
Sorry for my delay, I'm reviewing your PR now. Thanks for this :D

I agree with this solution. I this that is better keep plugins in composer.json file.

But, i have a question. In this solution can I remove the Reflection Plugin?

@ricardotulio
Copy link
Contributor Author

Hello @tacnoman, no, because Reflection Plugin will use get_declared_class and set it. In my opinion, it could be removed, but in another PR.

Add this files into composer.json just make this class declared, so Reflection Plugin can find it. In another way, the composer autoload just will load when someone try to instantiate, so Reflection Plugin will never find some plugin class.

@renatocassino renatocassino merged commit 80298c5 into renatocassino:master Jul 12, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants