Added a way to be forward compatible with version 2.0 #132
Conversation
|
hm. i see what you want to do. but is it worth it? are the separate plugins that complicated? i'd rather tag a last 1.x version of those and then move to 2.x. there should be no other things depending on those plugins, so the cost of having a major version seems minimal to me. |
|
HttplugBundle is also effected since it has a custom plugin and a custom client. |
|
I think this small BC layer is probably a good idea. Much less work for upgrading, and much less messy for trying to deal with 1.x and 2.x existing at the same time. Otherwise, we risk php-http ironically causing dependency conflicts and mess, with some people needing 1.x and others 2.x, purely because of the interface incompatibility of these 2 classes. |
|
On one hand I'm not sure it's worth it, on the other hand dependency conflicts already happen in the php-http ecosystem, so anything that makes it less possible should at least be considered. If it works, I'm okay with it. Side note: should we rather make these abstract classes traits? |
|
okay, okay, convinced me ;-) traits should work fine too: https://3v4l.org/ai0TG (and i tried, if we don't implement the abstract method, we get a php error that the abstract method is not implemented) do we want to change to a trait for this? what i like about the trait is that the plugins still explicitly implement Plugin, rather than implementing it indirectly by extending the base class. |
|
Traits are better. Sure! |
|
looks good to me. pretty-ci seems to be a bit flaky these days :-( |
|
No, it is our config file in master: https://github.com/php-http/client-common/blob/master/.php_cs This is fixed in 2.x |
| { | ||
| abstract protected function doHandleRequest(RequestInterface $request, callable $next, callable $first); | ||
|
|
||
| public function handleRequest(RequestInterface $request, callable $next, callable $first) |
There was a problem hiding this comment.
Isn't this missing the return type hint?
There was a problem hiding this comment.
This is for master (ie 1.9.0).
Here we should be compatible with httplug 1. Check #133 for httplug 2.0.
| { | ||
| abstract protected function doSendRequest(RequestInterface $request); | ||
|
|
||
| public function sendRequest(RequestInterface $request) |
To avoid updating all our plugins and the HTTPlug bundle, we can be forward compatible.
https://3v4l.org/pn47M
https://3v4l.org/2kgtG