Skip to content
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

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

Open
hacktivista opened this issue Mar 6, 2019 · 13 comments
Open

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

hacktivista opened this issue Mar 6, 2019 · 13 comments

Comments

@hacktivista
Copy link

hacktivista commented Mar 6, 2019

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
Copy link
Member

barryvdh commented Mar 8, 2019

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
Copy link
Member

barryvdh commented Mar 20, 2019

Okay so I've gathered:

  • The new adapters support both httplug and psr-18, so with new versions we can use both: Allow guzzle6 v2 adapter omnipay#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:

@judgej
Copy link
Member

judgej commented Mar 20, 2019

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
Copy link
Member

See #207

@hacktivista
Copy link
Author

@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 😄

* 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.

@barryvdh
Copy link
Member

The interface is the same, so it doesn't matter we support both packages.

@barryvdh
Copy link
Member

Actually, removing the typehint in the constructor isn't breaking (only in methods). So if we don't add the final, it should be fine.

@hacktivista
Copy link
Author

Plz check #208

@pkly
Copy link

pkly commented Jan 27, 2021

Any progress here? Using these interfaces is kinda unsightly.
I would highly advise for simply moving to PSR-18 fully.

@barryvdh
Copy link
Member

Which interfaces?

@pkly
Copy link

pkly commented Jan 28, 2021

Omnipay's HttpClient uses a custom interface

@reinos
Copy link

reinos commented May 10, 2023

Any update on this? Can we use PSR-18 in omnipay or do we need to wait on the MR #207 ?

@Axent96
Copy link

Axent96 commented Jun 19, 2023

It looks a bit similar with #207 && #262
Any plans for progress here?

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

No branches or pull requests

6 participants