- Notifications
You must be signed in to change notification settings - Fork524
Implement httplug#487
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
composer.json Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
It can be removed, httplug is now in a stable version :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Sadly no, because the discovery depends on puli which is in beta state currently 😢
joelwurtz commentedJan 29, 2016
You should ignore the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
One of the way we have done this for not having discovery, is to put discovery in suggest package then on the constructor we check the existence of the DiscoveryClass, if it has not been found we throw a \LogicException with a specific error which will be in our documentation for easy googling on that, here is an example :
if (null ===$responseFactory && !class_exists('\Http\Discovery\MessageFactoryDiscovery')) {thrownew \LogicException('No response factory provided and no discovery service is present to guess it, maybe you need to install php-http/discovery package?');}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I'm not sure about the discovery service at all, it brings many problems and puli as new dependency. Maybe we should wait for a stable release of the discovery service.
joelwurtz commentedJan 29, 2016
Also |
Baachi commentedFeb 1, 2016
It seems puli have a problem with hhvm and php7, because travis passes for php 5.5, 5.6 but not for php7 and hhvm. |
Baachi commentedFeb 22, 2016
Some notes: Currently the tests don't passes on PHP7 and HHVM because puli is sadly not compatible with that. |
| "php-http/mock-client":"^0.2", | ||
| "php-http/message":"^1.0", | ||
| "php-http/guzzle6-adapter":"^1.0", | ||
| "php-http/discovery":"^0.7.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Use "php-http/discovery": "^0.8.0" and put that in require.
smartpierre commentedJun 9, 2016
any updates ? |
Nyholm commentedJun 9, 2016
If there is interest from the maintainers to merge this, I will be happy to complete this PR. |
willdurand commentedJun 9, 2016
Yes, this should be merged into the lib at some point, especially because the current http lib is deprecated now. cc@egeloen |
Nyholm commentedJun 9, 2016
Great. I'll try to get some work this weekend and next week. @Baachi I hope you do not mind that I continue your work. |
Baachi commentedJun 10, 2016
Feel free to continue the work. |
willdurand commentedJun 12, 2016
See#512 |
I started to integrate HTTPlug into geocoder.
Current status: Its integrated but i'm not about some implentation details. For example i don't know if we really need the
php-http/discoverypackage because itintroduce a lot of packages for a small benefit.
Reviews and thoughts are welcome!
Fixes#484 and#480