algolia / algoliasearch-client-php

⚡️ A fully-featured and blazing-fast PHP API client to interact with Algolia.
https://www.algolia.com/doc/api-client/php/getting-started/
MIT License
671 stars 116 forks source link

Add support for psr/http-message ^2.0 #717

Closed Stoux closed 1 year ago

Stoux commented 1 year ago
Q A
Bug fix? no
New feature? yes
BC breaks? no
Related Issue ...
Need Doc update no

Describe your change

This introcues support for psr/http-message 2.0 while keeping support for 1.0.

The only change in psr/http-message:2.0 introduces return types. As we support php:^7.2 we can also introduce these to support both 1.0 & 2.0. This also realigns paramater names with their interface definitions to better support named arguments (introduced in php:^8.0).

What problem is this fixing?

Allows inclusion in projects that require psr/http-message:^2.0

Stoux commented 1 year ago

Guzzle 6 tests seem fail because of the lock that is generated, which resolved http-message to the latest (2.0), while guzzle 6 requires http-message (1.0). Works if you just run the require command without generating a .lock first.

Not sure about the code styling errors, as I've only modified existing lines. Was the code styling of those files already incorrect?

tobimori commented 1 year ago

Happy to see this merged as I'm currently running into this issue too! @DevinCodes

Stoux commented 1 year ago

I've fixed the code style errors. The only remaining is one outside the scope of this PR, otherwise they would have succeeded.

I've also added a commit that allows the Circle CI test to modify the locked dependencies (caused by the initial composer install) when installing Guzzle, this fixes the issue when Guzzle (6) requires http-message 1.0 while 2.0 is installed. Let me know if you want to revert this change.

damcou commented 1 year ago

Hello there;

Just a small heads up to confirm that this PR has been spotted and will be reviewed ASAP, probably after we investigate and fix the CI issues in August. Thanks a lot for submitting this.

damcou commented 1 year ago

Hello @Stoux . I used your branch to test and fix the CI on another one from our repo: https://github.com/algolia/algoliasearch-client-php/pull/719 , this includes all of your changes and it has been merged, thanks again ! Obviously, this will be mentioned in the next release notes ;)