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

chore(deps): allow psr/http-message v2 #223

Closed
wants to merge 13 commits into from

Conversation

simPod
Copy link
Contributor

@simPod simPod commented Apr 5, 2023

No description provided.

@nicolas-grekas
Copy link
Collaborator

In the CI:

  • Locking psr/http-message (1.1)

So that's not enough.

Also, is this lib really compatible with v2? Doesn't it miss some return types?

@simPod
Copy link
Contributor Author

simPod commented Apr 5, 2023

Hm, let's test it.

@simPod simPod marked this pull request as draft April 5, 2023 08:31
@simPod simPod force-pushed the psr/http-message branch 2 times, most recently from 8d096fb to f0aa0ba Compare April 5, 2023 08:46
@simPod
Copy link
Contributor Author

simPod commented Apr 5, 2023

There are indeed some type changes required but I believe the v2 support won't be possible now since old php versions are allowed.

@simPod simPod force-pushed the psr/http-message branch from f0aa0ba to 41f3804 Compare April 5, 2023 08:47
@nicolas-grekas
Copy link
Collaborator

We should bump to PHP >=7.2 also

@simPod
Copy link
Contributor Author

simPod commented Apr 5, 2023

If that's okay with you I've opened >=7.2 PR here #226

@simPod
Copy link
Contributor Author

simPod commented Apr 5, 2023

Also, I think we cannot have support for both v1 and v2 since methods with typed arguments are not backward compatible.

@nicolas-grekas
Copy link
Collaborator

methods with typed arguments are not backward compatible

because of the behavior change when passing incompatible types, or?

@simPod simPod force-pushed the psr/http-message branch from 41f3804 to 1f6a69e Compare April 5, 2023 11:43
@simPod
Copy link
Contributor Author

simPod commented Apr 5, 2023

Because changing e.g.

-     public function withProtocolVersion($version): self
+     public function withProtocolVersion(string $version): self

is required and compatible with v2 but not compatible with v1

@nicolas-grekas
Copy link
Collaborator

It's compatible with 1.1, so we could just bump to ^1.1|^2.0, no?

@simPod simPod force-pushed the psr/http-message branch from b787556 to e2e6a5e Compare April 5, 2023 11:48
@simPod
Copy link
Contributor Author

simPod commented Apr 6, 2023

Test suite is being adapted here php-http/psr7-integration-tests#68

@simPod simPod force-pushed the psr/http-message branch from 855be2e to 2008b4b Compare April 6, 2023 14:46
@simPod simPod force-pushed the psr/http-message branch from 2008b4b to c6c6e4e Compare April 6, 2023 14:51
@nicolas-grekas
Copy link
Collaborator

Closing in favor of #234, thanks for pushing for this.

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.

2 participants