Add support for symfony/http-client #139
Conversation
| @@ -104,7 +116,7 @@ public static function getCandidates($type) | |||
| } | |||
| } | |||
|
|
|||
| return $candidates; | |||
| return array_merge($candidates, self::$classes[Psr18Client::class]); | |||
There was a problem hiding this comment.
this ensures that the order of discovery is the same for httplug and psr18 clients
There was a problem hiding this comment.
with autodiscovery, it could be kind of tricky to change the ordering - people might rely on getting a specific client and this change could trip them up. if they refactor to psr-18 and then get a different client, i think that is less surprising than just updating discovery.
@Nyholm i think you wrote this originally. was the intention of the ordering to prefer the psr-18 clients when psr-18 is requested?
i would tend to revert this change. but can be convinced otherwise ;-)
There was a problem hiding this comment.
Since only buzz is listed explicitly in the psr18 list, this means ppl that have both buzz and something else will get the something else instead. People that don't have buzz won't see a difference.
But having Symfony before Guzzle when PSR18 is loaded, and Guzzle before Symfony when httplug is is more weird.
There was a problem hiding this comment.
right. plus when we add to the PSR18 list, we might prepend another client (like the symfony client, which has some chance of being installed unwittingly when upgrading dependencies...). well anyways, discovery is the yolo approach :-)
waiting for @Nyholm to give his opinion too.
There was a problem hiding this comment.
Yeah, My intention was to prefer PSR18 clients over other clients.
Just note: Our BC promise does not include "what client".
There was a problem hiding this comment.
reverted, I'll let you care in another PR if you do :)
| @@ -104,7 +116,7 @@ public static function getCandidates($type) | |||
| } | |||
| } | |||
|
|
|||
| return $candidates; | |||
| return array_merge($candidates, self::$classes[Psr18Client::class]); | |||
There was a problem hiding this comment.
Yeah, My intention was to prefer PSR18 clients over other clients.
Just note: Our BC promise does not include "what client".
Checklist