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

Psr17: replace usage of deprecated PHP-HTTP Factories with PSR-17 factories #439

Merged
merged 6 commits into from
Nov 21, 2023

Conversation

glaubinix
Copy link
Contributor

@glaubinix glaubinix commented Nov 16, 2023

Q A
Bug fix? no
New feature? no
BC breaks? yes
Deprecations? yes
Related tickets fixes #433
Documentation if this is a new feature, link to pull request in https://github.com/php-http/documentation that adds relevant documentation
License MIT

What's in this PR?

Replaces usage of PHP-HTTP factories with PSR-17 factories

Why?

PHP-HTTP factories are deprecated

Checklist

  • Updated CHANGELOG.md to describe BC breaks / deprecations | new feature | bugfix
  • Documentation pull request created (if not simply a bugfix)

To Do

  • Remove remaining references to the PHP-HTTP factories
  • Remove php-http/message-factory from composer.json

Questions

  • guzzle5-adapter doesn't support psr-17 and seems abandoned. Is it ok to drop support for it?

@glaubinix
Copy link
Contributor Author

Considering that the guzzle5-adapter is basically abandoned, I would suggest removing support for it here too.

@glaubinix glaubinix force-pushed the psr17-18 branch 8 times, most recently from f14f589 to 3ecdcb2 Compare November 17, 2023 22:44
@dbu
Copy link
Collaborator

dbu commented Nov 18, 2023

thanks a lot for this work! i agree we should do this, and do BC breaks. and i also agree with dropping guzzle 5 support. we could even also drop guzzle 6 i think and very old php versions.

because of the major BC breaks, this seems the moment to bump the bundle to version 2. i just created the 2.x branch - can you please rebase and then add to the changelog the necessary informations of what changed / what users need to adjust in their configuration?

do you see other cleanups or changes that we should do before tagging 2.x? it would be the opportunity to get rid of legacy things and make necesary BC breaks.

@glaubinix
Copy link
Contributor Author

Rebased the PR onto 2.x and added changelog entries.

I see you already have a PR for marking classes as final and resolving some more deprecations. The other big deprecation I saw is the HttpClient type hint that needs to be replaced with ClientInterface and AutoDiscoveryFactory to use the new discovery factory. Happy to send a PR for that once this PR is done.

Optional changes should probably be based on how much easier it will make your life as a maintainer:

  • guzzle6 (still works without much change, so up to you)
  • drop support for old PHP versions, could then also drop Symfony 4.4 which isn't supported anymore either

@dbu dbu merged commit 47bb000 into php-http:2.x Nov 21, 2023
12 checks passed
@dbu
Copy link
Collaborator

dbu commented Nov 21, 2023

awesome, thanks a lot! i merged, as this is a nice package of changes.

if you are motivated to do it, i think raising to PHP 8 would be really neat so we can modernize the code as well. dropping support for legacy symfony versions would also be good - there is some ugly stuff in src/Collector/PluginClientFactoryListener.php for example to support legacy versions.

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 this pull request may close these issues.

Package php-http/message-factory is abandoned, you should avoid using it. Use psr/http-factory instead
2 participants