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

Add dependency on guzzlehttp/psr7 #692

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

tomcastleman
Copy link

@tomcastleman tomcastleman commented Aug 25, 2021

Fixes

Since #689 the minimum version of guzzlehttp/psr7 should be^1.7.0 due to the use of GuzzleHttp\Psr7\Query.

However the dependency of guzzlehttp/psr7 is currently only inferred through the dependency for guzzlehttp/guzzle:^6.3 || ^7.0.

Should guzzlehttp/guzzle resolve to version 6.5 (as at the time of writing), the inferred minimum version of guzzlehttp/psr7 is only ^1.6.1. In my case I had another dependency declaring guzzle/psr7:<1.7 and so the issue did not become apparent during composer update, but only via a runtime exception in the app when the Query class was not found.

Technical details:

  • twilio-php version: 6.28.0
  • php version: 8.0.9

Checklist

  • I acknowledge that all my contributions will be made under the project's license
  • I have made a material change to the repo (functionality, testing, spelling, grammar)
  • I have read the Contribution Guidelines and my PR follows them
  • I have titled the PR appropriately
  • I have updated my branch with the main branch
  • I have added tests that prove my fix is effective or that my feature works
  • I have added the necessary documentation about the functionality in the appropriate .md file
  • I have added inline documentation to the code I modified

If you have questions, please file a support ticket, or create a GitHub Issue in this repository.

@GrahamCampbell
Copy link

Guzzle maintainer here. It is not necessary to ask for 1.7.0. ^1.6 || ^2.0 is best, so long as you use the modern static class method API, rather than the legacy functions API. It was added in 1.6.0.

@GrahamCampbell
Copy link

If you wanna sure the versions installed are definitely PHP 8.1 compatible, you can use ^1.8.3 || ^2.1.

@phpfui
Copy link
Contributor

phpfui commented Feb 8, 2022

@tomcastleman Take the suggestion from @GrahamCampbell and update the psr7 dependency. This will help with 8.1 compatibility. You will also have to remove the theseer/phpdox dependency, as it was removed after your branch was made.

I just got the other 8.1 fixes merged, so if you get this merged, then 8.1 will be fully supported. Then @shwetha-manvinkurke can merge and release an SDK fully supporting 8.1, which will be awesome for us cutting edge people.

@GrahamCampbell
Copy link

#776 replaces this PR.

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.

4 participants