-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Allow composer autoloading (to coexist) #9407
Allow composer autoloading (to coexist) #9407
Conversation
aeadc74
to
eaf9adf
Compare
PR is done and does not intact the current autoloading in any way. However, Roundcube/plugins can be newly easily used using composer as deps 🎉🎉🎉 |
Looks good. I wonder whether we could do something that does not look like a hack regarding that change in |
I do not support this. The reason is I belive it should be initialized for every environment (web & CLI). Here we solve the first composer install/update only. The reason is composer classmap autoloader has not discovered the classes yet (when the roundcube plugin installer composer plugin is called). For consequent run, the class is autoloadable/present, thus disabling this for CLI in general is not needed and IMO not wanted. |
It would also break oauth tests. So, maybe the |
27b349b
to
f6be053
Compare
@alecpl now this PR is complete and fully BC. Please merge it and submit this repo to Packagist. I will then take care of the remaining changes. |
ps. |
Thank you very much ❤ I did a quick verification using:
and all works as expected incl. plugins 🎉🎉 |
extracted from #9241
This PR is 100% BC and does not imply any functional change.
This PR allows composer autoloading to address the goals from #9241, but keeps the Roundcube autoloading intact.
Later, the transition to composer autoloading can be discussed in the original #9241 with minimal LoC needed.