-
Notifications
You must be signed in to change notification settings - Fork 46
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
Replace Guzzle with generic PSR18 discovery #99
base: master
Are you sure you want to change the base?
Replace Guzzle with generic PSR18 discovery #99
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.
I've only spend a little bit of time so far on the tests, but haven't managed to work out how to get them running either. I'm not that familiar with PSR-17/18 so did some reading about them too, and the supporting packages. I'll spend more time on this soon.
I have a feeling this is the part that isn't being mocked correctly in the PHPSpec tests:
$this->httpClient = new HttpMethodsClient(
$httpClient,
Psr17FactoryDiscovery::findRequestFactory(),
Psr17FactoryDiscovery::findStreamFactory()
);
*/ | ||
protected function postRequest(string $url, array $options): string | ||
protected function postRequest(string $url, array $headers = [], string|StreamInterface|null $body = null): string |
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 branch currently still supports PHP 7.4, which doesn't let us use union types. Since PHP 7.4 is EOL, I think we can bump to ^8.1
and keep these types.
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 good. I've made this change
1094433
to
a338f6f
Compare
Sorry, I haven't had time to look into this again, but it is on my mind. |
No worries, there's no rush. |
@GuySartorelli I'm not sure when I will have time to look more into this |
That's fine, I don't have any real need for this - just saw an opportunity to tidy up this package's dependencies and figured I'd give it a go. |
Resolves #91
Note that this would most likely need to be a new major release, as it changes the method signature of several methods which were previously relying on a Guzzle-specific options array.
This works well locally, but I have never used phpspec before and I don't know how to update the tests. Can someone with some knowledge of phpspec please give me some guidance?
All of the
ClientSpec
tests are failing - the error message and trace is: