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

3.x preparation : Make httplug optional, depends on psr18 #43

Merged
merged 4 commits into from Feb 16, 2019

Conversation

Projects
None yet
2 participants
@joelwurtz
Copy link
Member

joelwurtz commented Jan 21, 2019

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

@dbu
Copy link
Contributor

dbu left a comment

thanks!
you need to adjust the branch-alias in composer.json

Show resolved Hide resolved CHANGELOG.md Outdated
Show resolved Hide resolved CHANGELOG.md Outdated
* @param array $options
*/
protected function assertResponse($response, array $options = [])
protected function assertResponse(ResponseInterface $response, array $options = [])

This comment has been minimized.

@dbu

dbu Jan 22, 2019

Contributor

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.

This comment has been minimized.

@joelwurtz

joelwurtz Jan 22, 2019

Author Member

Yes, php will fail before that so we are pretty safe here

@@ -15,13 +15,18 @@
}

This comment has been minimized.

@dbu

dbu Jan 22, 2019

Contributor

the branch alias in the composer.json file needs to be updated

This comment has been minimized.

@joelwurtz

joelwurtz Jan 22, 2019

Author Member

👍 totally forget this comment my bad, should be good

@joelwurtz

This comment has been minimized.

Copy link
Member Author

joelwurtz commented Jan 29, 2019

Are we good to merge this ?

@joelwurtz

This comment has been minimized.

Copy link
Member Author

joelwurtz commented Feb 3, 2019

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.

"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",

This comment has been minimized.

@dbu

dbu Feb 4, 2019

Contributor

lets use ^7.1 here, we don't know if we support php 8...

@dbu

This comment has been minimized.

Copy link
Contributor

dbu commented Feb 4, 2019

looks good to me except the php version constraint. can you update that and then merge this into master?

@joelwurtz

This comment has been minimized.

Copy link
Member Author

joelwurtz commented Feb 16, 2019

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)

@joelwurtz joelwurtz merged commit 48bfe2d into master Feb 16, 2019

2 of 4 checks passed

continuous-integration/travis-ci/pr The Travis CI build could not complete due to an error
Details
continuous-integration/travis-ci/push The Travis CI build could not complete due to an error
Details
continuous-integration/styleci/pr The analysis has passed
Details
continuous-integration/styleci/push The analysis has passed
Details

@joelwurtz joelwurtz deleted the feature/psr18-only branch Feb 16, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment