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

Support PSR-17 and PSR-18 interfaces and php-http 2.x packages #41

Closed
mxr576 opened this issue Jan 22, 2019 · 0 comments · Fixed by #106
Closed

Support PSR-17 and PSR-18 interfaces and php-http 2.x packages #41

mxr576 opened this issue Jan 22, 2019 · 0 comments · Fixed by #106
Assignees

Comments

@mxr576
Copy link
Contributor

mxr576 commented Jan 22, 2019

Preface

First PSR-17 and later PSR-18 accepted and PHP-HTTP libraries has started to update their code to rely on new interfaces and add return types to their methods. New major versions got released from these libraries because these changes caused BC breaks with the previous versions.

Problem

Libraries that we use:

  • php-http/httplug: php-http/httplug@v1.1.0...v2.0.0
    • We hare already used these return types so these are no BC breaking changes for this library.
  • php-http/client-common: php-http/client-common@1.9.0...2.0.0-RC1 (only RC1 is available at this moment)
    • Good news, this upcoming version is finally contains a proper fix for Path prefix must be present php-http/client-common#113 (implemented in Dont add path to an url we already added a path to. php-http/client-common#141). Bad news, this fix is only available in the 2.x branch and it can not be back-ported officially to the 1.x branch. We could back-port this patch for this library but applying it would be only required if < 2.0.0 version is installed from php-http/client-common. Cweagans' composer patches (that we use and recommend) does not support version based patch restrictions (only vaimo's version has that feature) therefore applying this patch could not be automated. Patching the library would cause extra complexity for consumers. Consequently, it would be better to set the minimum requirement to the php-http/client-common:^2.0.
    • For the first sight, this new major release only requires some small changes in this library. Although, if you check these changes more closely then you can see they makes impossible to support both the 1.x version and the 2.x version from the library in the same time.
      • Different exception classes are being used in the Journal and History classes in the 1.x and 2.x version: Updated the exceptions we are using php-http/client-common#163.
        • This could be fixed by not extending the Journal interface from the client-common in this library's JournalInterface interface and creating our own History plugin that depends on this library's JournalInterface and not client common's definition. That would cause an additional maintenance cost for us.
        • Or by implementing something like Nyholm suggeted: Release 2.0 php-http/client-common#115 (comment)
  • php-http/guzzle6-adapter: php-http/guzzle6-adapter@v1.1.1...v2.0.1 This is only a dev dependency so it should not be a blocker, just added to the list for the sake of completeness.

Related conversion which contains some info about the maintenance and support of the 1.x (older) php-http packages and more: php-http/client-common#115 (comment)

Proposed solution

Because it seems there is no good way to support both 1.x and 2.x version from all required php-http libraries in the current 2.x branch therefore we should release a new major from this library. The new 3.x version should rely on PSR-17 and PSR-18 interfaces instead of interfaces provided by php-http/httplug.

New client implementations and consumers should use the PSR-18 interfaces directly. In the long term, we expect PSR-18 to completely replace the need for HTTPlug.

(From php-http/httplug's README.md)

The new 3.x version should support only 2.x versions from php-http/client-common and php-http/guzzle6-adapter (as dev dependency) libraries.

In addition, it seems we should wait for the release of the 1.6.0 from the php-http/discovery and depend on that: php-http/discovery#134.

The same resolution happens in case of the Mailgun library: mailgun/mailgun-php#522

Questions

  1. What should happen with the 2.x version? Should we abandon it completely and force developers to update to the 3.x version or mark it with a similar status like Drupal's "Maintenance fixes only". ? If we choose the second option, how long we should wait until we mark it as unsupported?

(This is a follow up on #24.)

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 a pull request may close this issue.

2 participants