Skip to content

Make sure we can use PSR-18 clients in version 1.x #143

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

Merged
merged 2 commits into from
Dec 31, 2018
Merged

Make sure we can use PSR-18 clients in version 1.x #143

merged 2 commits into from
Dec 31, 2018

Conversation

Nyholm
Copy link
Member

@Nyholm Nyholm commented Dec 30, 2018

Q A
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Related tickets
Documentation
License MIT

Our 2.x branch is compatible with PSR-18. The master branch should also be compatible.

Ive ipdated all places where we are consuming a httplug client. They do now support both httplug 1.x and PSR-18.

FYI @gmponos

@gmponos
Copy link
Contributor

gmponos commented Dec 31, 2018

Just throwing a long shot idea here.. on v1 what if there was a PSR18 HTTPlug adapter?

I think someone else has also suggested this but I am lost with all these issues..

@Nyholm
Copy link
Member Author

Nyholm commented Dec 31, 2018

That would not help. Because PSR-18 has the exact same signature but with PHP7 return values.

That is why all these updates to 2.x are super simple. They are just dropping PHP5 and adding return values all over the place.

Copy link
Contributor

@gmponos gmponos left a comment

Choose a reason for hiding this comment

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

Not sure if we must have tests here as well.. rather than that it seems fine.

Copy link
Contributor

@dbu dbu left a comment

Choose a reason for hiding this comment

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

thanks!

@dbu dbu merged commit 19c28ed into master Dec 31, 2018
@dbu dbu deleted the patch-psr18 branch December 31, 2018 09:50
@Nyholm
Copy link
Member Author

Nyholm commented Dec 31, 2018

Thank you for merging

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.

4 participants