PSR-18 is already released, but not implemented on Http\Client

3ynm commented 5 years ago

Omnipay\Common\Http\Client says that it will be changed to comply with PSR-18 when released. It's been released in last october. So it's just a reminder.

I must clarify, I don't know if it's needed, just saw this on the sourcecode.

barryvdh commented 5 years ago

It's released but we'll let it settle for a while to see if there is large support for it, and if we can change without breaking BC.

barryvdh commented 5 years ago

Okay so I've gathered:

The new adapters support both httplug and psr-18, so with new versions we can use both: https://github.com/thephpleague/omnipay/pull/556
We provide our own interface, so for gateways it doesn't matter.
We use discovery + require the httplug implementation. With 1.6 of discovery, it's possible to discover psr-18 client the same way (see https://php-http.readthedocs.io/en/latest/discovery.html#psr-18-client-discovery)
We provide a default adapter in league/omnipay, but not with common. So requiring an psr-18 implementation instead of httplug would be breaking perhaps, but Composer would notify users about this, so I guess it's not a big problem?
- We haven't typehinted the client for this reason, as the signature is also the same it shouldn't be a problem. But the PSR-7 Factory is deprecated and that is typehinted.. The signature is similar so I guess we could remove the typehint as long no one extends the Client.

So to upgrade:

[x] Require adapter that support psr-18 in league/omnipay: https://github.com/thephpleague/omnipay/pull/556
[x] Require adapter that support psr-18 in omnipay/tests
[ ] Require psr-18 implementation, upgrade our HTTP client to use PSR-18. (#207)
[ ] Require a psr-17 implementation, or require it ourself? (#207)

judgej commented 5 years ago

I'm playing with PSR-15, PSR-17 and PSR-18 on a Xero API at the moment to understand how it all works, and it seems good so far.

barryvdh commented 5 years ago

See #207

3ynm commented 5 years ago

@barryvdh

* The new adapters support both httplug and psr-18, so with new versions we can use both: [thephpleague/omnipay#556](https://github.com/thephpleague/omnipay/pull/556)
We could add deprecation notices?, so we don't have to bother supporting old packages when jumping to V4
* We provide our own interface, so for gateways it doesn't matter.
I think it should. PSRs are proposed as an "standarized framework", so you don't need to learn the workings of different APIs on every project. I think we should point towards usage of PSR only interfaces in case of HTTP, but not breaking compatibility without a major version increment.
* We use discovery + require the httplug implementation. With 1.6 of discovery, it's possible to discover psr-18 client the same way (see https://php-http.readthedocs.io/en/latest/discovery.html#psr-18-client-discovery)
Nice :smile:
* We provide a default adapter in `league/omnipay`, but not with common. So requiring an psr-18 implementation instead of httplug would be breaking perhaps, but Composer would notify users about this, so I guess it's not a big problem?
Well... maybe it will be painful for some people and is not an smart movement if you don't pretend to increase the major version number in league/omnipay... I think we should not break anything, but add support to new functionality so we increase only a minor version number in this package (3.1.0).
* We haven't typehinted the client for this reason, as the signature is also the same it shouldn't be a problem. But the PSR-7 Factory is deprecated and that is typehinted.. The signature is similar so I guess we could remove the typehint as long no one extends the Client.
Again, deprecation notices would be great. I think having an extensible client is sweet.