chore: remove php-http/socket-client<2.0 support #418
Conversation
|
This is incomplete. We can pass array there only for >=2.0, so we need to add conflict section |
qkdreyer
force-pushed
the
patch-2
branch
2 times, most recently
from
867c94b to
69e2faa
Compare
qkdreyer
changed the title
|
fixed the CI |
|
This is not solving deprecation now, though. Why not sticking to original solution and just add conflict section? |
|
I’m not sure how you would like to implement that… using try/catch or reflection ? |
src/ClientFactory/SocketFactory.php
Outdated
| @@ -31,6 +31,6 @@ public function createClient(array $config = []) | |||
| throw new \LogicException('To use the Socket client you need to install the "php-http/socket-client" package.'); | |||
| } | |||
|
|
|||
| return new Client($this->messageFactory, $config); | |||
| return new Client($this->messageFactory, $config, $config); | |||
There was a problem hiding this comment.
thanks for looking into this.
i think the actual bug here is in the socket client. the aim was to be BC with version 1, but deprecate passing that factory. the signature of version 1 was correct for how we instantiated the client: https://github.com/php-http/socket-client/blob/v1.4.1/src/Client.php#L52
version 2 should at https://github.com/php-http/socket-client/blob/47e68cade6b76e7fcaaf648048845d9dde0accf9/src/Client.php#L61 should use $config2, not $config which i don't see how it could be useful...
i suggest we fix the socket client to work as intended. we could additionally add a version constant into the socket client to allow the bundle to call the constructor in a future proof way and avoid the deprecation warning.
There was a problem hiding this comment.
@joelwurtz what do you think, would it make sense that way?
you could mark a conflict with socket-client < 2.0 in the composer section. then new versions of this bundle can only be installed with a socket-client version 2, but not version 1. i think that is the best solution for the bundle. we should also fix the BC logic of the socket client to actually work as intended, but thats a different repo. |
|
@qkdreyer do you have time to update this? |
qkdreyer
force-pushed
the
patch-2
branch
2 times, most recently
from
963a4a9 to
dd48646
Compare
qkdreyer
changed the title
qkdreyer
changed the title
|
the socket client is now also fixed in version 2.1.1 to have the intended BC with version 1. |
https://github.com/php-http/socket-client/blob/master/src/Client.php#L51