3.x preparation : Make httplug optional, depends on psr18 #43
Conversation
| * @param array $options | ||
| */ | ||
| protected function assertResponse($response, array $options = []) | ||
| protected function assertResponse(ResponseInterface $response, array $options = []) |
There was a problem hiding this comment.
the explicit assertion would lead to a phpunit failure instead of a full php type explosion. but the client interface has a return type declaration, so it should be impossible to ever have anything else than a ResponseInterface, right? if thats the case then the change is fine for me.
There was a problem hiding this comment.
Yes, php will fail before that so we are pretty safe here
| @@ -15,13 +15,18 @@ | |||
| } | |||
There was a problem hiding this comment.
the branch alias in the composer.json file needs to be updated
There was a problem hiding this comment.
👍 totally forget this comment my bad, should be good
|
Are we good to merge this ? |
|
Tests failure is expected, (as library need to update and do changes to fix their tests with the new version), can we merge this ? It's a blocker for advancing on socket-client v2. |
composer.json
Outdated
| "php": ">=5.4", | ||
| "phpunit/phpunit": "^4.8.35 || ^5.4.3 || ^6.0 || ^7.0", | ||
| "php-http/httplug": "^1.0 || ^2.0", | ||
| "php": ">=7.1", |
There was a problem hiding this comment.
lets use ^7.1 here, we don't know if we support php 8...
|
looks good to me except the php version constraint. can you update that and then merge this into master? |
|
PHP Version constraint updated, tested those change with socket-client and works fine, will merge it, use dev-master in socket-client ATM and release a new version once everything looks ok (i don't think there is anything to add but just in case) |
This allow library relying on psr18 only to use this tests with requiring httplug 2.0 (but it would still be compatible)
We should create a 3.x version after this is merged