Skip to content

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

Merged
merged 6 commits into from
Dec 29, 2018
Merged

Support PSR-18 #135

merged 6 commits into from
Dec 29, 2018

Conversation

Nyholm
Copy link
Member

@Nyholm Nyholm commented Dec 28, 2018

Q A
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Related tickets
Documentation
License MIT

What's in this PR?

We should show that we support PSR-18. I've replaced Http\Client\HttpClient with Psr\Http\Client\ClientInterface on all the places where we consume a client.

This will fix #122

Copy link
Member

@sagikazarmark sagikazarmark left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds reasonable

@Nyholm
Copy link
Member Author

Nyholm commented Dec 29, 2018

Thank you for the review

Copy link
Contributor

@dbu dbu left a 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 👍

*/
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');
Copy link
Contributor

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

@@ -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())
Copy link
Contributor

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?

Copy link
Member Author

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.

@Nyholm
Copy link
Member Author

Nyholm commented Dec 29, 2018

This will fix #122

@Nyholm Nyholm added this to the v2.0.0 milestone Dec 29, 2018
@dbu dbu merged commit 53bddb2 into 2.x Dec 29, 2018
@dbu dbu deleted the 2.x-psr18 branch December 29, 2018 08:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants