-
Notifications
You must be signed in to change notification settings - Fork 53
Support PSR-18 #135
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
Support PSR-18 #135
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds reasonable
Thank you for the review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
very nice. great that the psr turned out to be compatible 👍
src/FlexibleHttpClient.php
Outdated
*/ | ||
public function __construct($client) | ||
{ | ||
if (!($client instanceof HttpClient) && !($client instanceof HttpAsyncClient)) { | ||
if (!($client instanceof ClientInterface) && !($client instanceof HttpAsyncClient)) { | ||
throw new \LogicException('Client must be an instance of Http\\Client\\HttpClient or Http\\Client\\HttpAsyncClient'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should adjust the exception message too
src/Plugin/AddPathPlugin.php
Outdated
@@ -47,7 +47,8 @@ public function handleRequest(RequestInterface $request, callable $next, callabl | |||
$identifier = spl_object_hash((object) $first); | |||
|
|||
if (!array_key_exists($identifier, $this->alteredRequests)) { | |||
$request = $request->withUri($request->getUri() | |||
$request = $request->withUri( | |||
$request->getUri() | |||
->withPath($this->uri->getPath().$request->getUri()->getPath()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not sure that this is more readable than the previous formatting. should we indent this some more? or extract the argument on this line to a variable so we can put $request->getUri()->withPath($prefixedPath)
on one line?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this change must have been suggested/automatic.
I like your suggestion though.
This will fix #122 |
What's in this PR?
We should show that we support PSR-18. I've replaced
Http\Client\HttpClient
withPsr\Http\Client\ClientInterface
on all the places where we consume a client.This will fix #122