-
Notifications
You must be signed in to change notification settings - Fork 83
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
feat: update guzzle to version 7 #199
feat: update guzzle to version 7 #199
Conversation
@gsteel Do you see any problem with releasing this as Version Should I drop |
According to Travis, Guzzle was installed at ^7.0 - if you can't test ^6 in CI, how do we know it works with this patch? I'd just drop ^6, release a minor and be done with it. You will have to update the lockfile, yes. |
Dropped support for Guzzle 6 in 93eaf2e |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The doc block additions duplicate the method signatures, but otherwise LGTM 👍 - The @final
suggestion will pay dividends in future - I'm a final by default sort of person, but it's just opinion. My argument is that it's easy to relax this stuff in future if there's good reason, but hard to tighten things up without breaking BC
Co-authored-by: George Steel <george@netglue.uk>
I added your suggestion. Should I merge and release now? |
You're the boss 😉 |
As raised in Issue Upgrade to Guzzle 7.x #170 Guzzle 7 is currently the most requested feature by the community.
This Pull-Request combines and improves changes from:
During local testing it was possible to support both Guzzle six and seven.