Skip to content
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

require all necessary psr implementations #63

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

dbu
Copy link

@dbu dbu commented May 21, 2022

the code relies on psr7, psr17 and psr18 implementations to be available, with the discovery.

it no longer uses the php-http httplug interface (which was made obsolete by psr-18). php-http/client-common is also not used in the code.

composer.json Outdated
"ext-json": "*"
},
"require-dev": {
"php-http/guzzle6-adapter": "^2.0",
"guzzlehttp/psr7": "^2.0",
Copy link
Author

Choose a reason for hiding this comment

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

i suggest to use the guzzle psr 7 implementation when using the guzzle client. but we can also switch back to nyholm, it should not break anything either way.

Copy link
Author

Choose a reason for hiding this comment

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

ok, with guzzle 6, we would need psr7 version 1 which does not include the factories. i suggest going with guzzle 7 for the tests. that is also compatible with php 8.

the code relies on psr7, psr17 and psr18 implementations to be available, with the discovery.

it no longer uses the php-http httplug interface (which was made obsolete by psr-18). php-http/client-common is also not used in the code.
@@ -25,5 +26,8 @@
"psr-4": {
"MailerLiteApi\\Tests\\": "tests/"
}
},
"conflict": {
"php-http/httplug": "< 2"
Copy link
Author

Choose a reason for hiding this comment

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

adding this conflict to be sure to not break for users who provide their own httplug client implementation. the httplug 2.* interface extends the psr-18 interface to be compatible with both.

@dbu
Copy link
Author

dbu commented May 21, 2022

i tried running the tests locally, but they fail because the tests assume the json is decoded to array, but RestClient::handleResponse does json_decode with false, so creating stdClass objects.

i don't think this has anything to do with the changes i propose. but it would be a good idea to fix the tests.

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.

None yet

1 participant