-
-
Notifications
You must be signed in to change notification settings - Fork 98
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
Update Goutte to version 4 removing the GuzzlePHP dependency #349
Conversation
Thanks for the PR! If you could target the 3.0 branch that would be great and I can go ahead and review it for merging. I believe it should also be a step forward to resolve #323 |
3a23e88
to
8a57e2e
Compare
@irfan-dahir I rebased it to 3.0.0. I'm also running this on PHP version 8.0 and it's running fine. Do you want me to make a separate PR that enables PHP 8.0 for this package? |
Thanks @rjd22 , I'll test this out and let you know! As for 8.0, let's put it on hold for a bit. I need to make sure the tests and Lumen framework both support it. Edit: Glad to know it's working fine on 8.0 tho! |
Perhaps add suggests for symfony http client and guzzle. |
@janvernieuwe sadly the Guzzle version is incompatible with the SymfonyHttpClient version. The upgrade is one way. I think dropping guzzle completely for this library on a major version is reasonable. |
@irfan-dahir would it be possible to get this one merged? The old Guzzle version would be really blocking for using the new 3.0 release. |
@rjd22 For sure - I will review and merge it before the end of the week. |
Thank you @irfan-dahir ! |
The PHPUnit tests seem to not work anymore so I didn't update them but I did an integration test on my own any it seems to work correctly. I'm just making this PR to share my change.
This is a breaking change I would say so you might want me to target it to the 3.0 branch.