-
-
Notifications
You must be signed in to change notification settings - Fork 174
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
Shipped Guzzle
client causes dependency hell
#542
Comments
Guzzle isn't a direct dependency of this app, though: https://github.com/nextcloud/bookmarks/blob/master/composer.json
|
Ah, I explicitly used libs that depended on guzzle 5 in order to be able to use them in nextcloud. When was Guzzle v6 introduced in nextcloud? It would be nice to be able to still use the app in v13 |
Generally speaking, I would say duplicate libraries should be avoided at any price, simply because code becomes completely unpredictable otherwise as the actually loaded class depends on the enabled apps and the order of the autoloader registration. cc @rullzer |
With the visual restructuring required for nc14, I'm afraid this could be a challenge. |
Best would be:
|
sigh Long live PHP 🎉 @rullzer Sounds good. |
0.12.x breaks nextcloud v14 RC1 because of guzzlehttp |
It's an interesting discussion here. Does that mean app devs shouldn't use any indirect dependency in an app that is also somehow in 3rdparty? :( In this specific case perhaps it makes sense updating the bookmarks dependencies, as their current versions rely on guzzle 6 instead of 5. @rullzer what do you think? |
In the future, it would be cool if core could provide a client that follows e.g. the psr-18 spec, so interoperability is maintained. |
That's probably advisable. |
Bookmarks v0.13 is released with Nc 14 support |
Steps to reproduce
IClientService
.client->get(…)
.Expected behaviour
Have a working http client.
Actual behaviour
💥
Server configuration
Nextcloud version:
14.0.0 Beta 2
Bookmarks version:
master
Logs
cc @skjnldsv because you originally reported this to me. Welcome to the php dependency hell 😈
The text was updated successfully, but these errors were encountered: