Allow psr/http-message ^2.0 #542
Conversation
Requires php: ^7.2 || ^8.0 Closes reactphp#513
|
@clue @WyriHaximus would it be possible to get some eyes on this and released if all good? |
composer.json
Outdated
| @@ -29,7 +29,7 @@ | |||
| "php": ">=7.1", | |||
| "evenement/evenement": "^3.0 || ^2.0 || ^1.0", | |||
| "fig/http-message-util": "^1.1", | |||
| "psr/http-message": "^1.0", | |||
| "psr/http-message": "^2.0 || ^1.0", | |||
There was a problem hiding this comment.
The only way this will work is if we go:
| "psr/http-message": "^2.0 || ^1.0", | |
| "psr/http-message": "^2.0", |
Reason for this is that we have a bunch of messages in https://github.com/reactphp/http/tree/3.x/src/Message that don't have a return type on those methods. If we'll allow 2.0 without adding that you'll get fatal errors, so IMHO we should switch to 2.0 and add the return types to all the relevant classes. /cc @clue
There was a problem hiding this comment.
@WyriHaximus #541 was not what you expect?
There was a problem hiding this comment.
@L3o-pold Aside from a few things, yes that should have worked.
There was a problem hiding this comment.
@WyriHaximus can you clarify what so we can move forward?
There was a problem hiding this comment.
@valzargaming I have opened a PR against your branch to add the minimum required to make this update work. I had to remove clue/http-proxy-react because it is only required for 1 example, and it eventually requires ringcentral/psr7 which in turn depends on "psr/http-message": "^1.0". You will need to rebase after merge because your branch is 2 commits behind and I didn't want to introduce a merge commit.
There was a problem hiding this comment.
@valzargaming sorry, one more thing. I didn't realize the CI ran against 7.1, you will need to remove https://github.com/reactphp/http/blob/3.x/.github/workflows/ci.yml#L22
There was a problem hiding this comment.
No worries. I don't have access to my PC right now, but if you make a PR for it I'll merge it.
There was a problem hiding this comment.
@L3o-pold As mentioned in clue/reactphp-http-proxy#64 (comment) it boils down to this function call: https://github.com/clue/reactphp-http-proxy/blob/1.x/src/ProxyConnector.php#L187 Mento to respond to you earlier but life got in the way. Had a look at fixing it but didn't get far
Adding the bare minimum to make psr/http-message ^2.0 work
Remove 7.1 from the CI matrix because psr/http-message ^2.0 requires 7.2 or later.
|
Hi, what is the latest on this PR? Will this be merged? This is creating an incompatibility with the AWS SDK which requires 2.0 Thanks. |
|
@marcelfolaron I believe that is the goal. I think the part holding us back right now is the PR to clue/reactphp-http-proxy which would allow us to keep that examples/11-client-http-proxy.php in the codebase. |
Requires php: ^7.2 || ^8.0, but still allowing usage of ^1.0 maintains backwards compatibility with the current PHP requirements
Closes #513