Provide PHP 8.0 compatibility #335
Conversation
The packaging gymnastics to support Guzzle 6 + 7 is only needed if PHP 7.1 support is still desired. Guzzle 6 supports PHP <= 7.4 and Guzzle 7 supports PHP 7.2 - 8.0.
There was a problem hiding this comment.
Thank you for this @edudobay! This is super useful!
Guzzle's packaging strategy has been a source of pain in this repo for a long time 😅 I think they are doing a great job, but for libraries like this one it can add unneeded constraints. It's the reason why we moved to HTTPlug, trying to be less strict on anybody using this library.
I wonder if this is our chance to move our tests to a more lightweight adapter (ie php-http/curl-adapter) for the tests. That would probably prevent us from having to deal with this again in the future. What do you think?
I also see your other point about 7.1 support. I think dropping support for 7.1 makes sense, its end of life was reached almost a year ago, and if that reduces the complexity of this change, I'm all in for it!
- Use the error plugin to check for error responses — this is the default setting when a HTTP client is not explicitly set. - Remove 'testExtendedClient' because it tests pure Guzzle functionality. This test was added when this library was coupled to Guzzle, but now it doesn't add any value over 'testBasicClient'.
|
Hi @GabrielAnca, that seems like a good idea! Following your suggestion, I refactored the tests to change the adapter, but I used With this new change, there's no conflict with PHP 7.1 and I see no obvious need to drop it. |
There was a problem hiding this comment.
Sounds great @edudobay! I think it looks much cleaner now, thank you for taking the time for this! 🎉
I also agree there's no need to remove PHP 7.1 for now given that it's not creating any issue anymore 😄 I also think there's no blocker from shipping this PR now, we can come back and add PHP 8 to the circleci config as soon as the image becomes available.
I'm having a look to fix the CircleCI builds, they are not kicking off 😬
|
I think I managed to fix the CircleCI builds! If you push an empty commit to this PR it should build. As soon as the four checks are green I'll approve it and merge it! |
|
Hi @GabrielAnca! An empty commit didn't seem to work. What else can we try? |
|
Seems all right for #336! Should I close this PR? |
|
@GabrielAnca I tried to copy your branch to mine and it worked! :D |
|
Sorry @edudobay totally forgot to reply yesterday. Happy that it worked! I'll try to get this one and a couple more merged to release a new version this week 😄 |
Why?
Provide compatibility with PHP 8.0 (released yesterday).
How?
Guzzle 6 supports PHP <= 7.4 and Guzzle 7 supports PHP 7.2 - 8.0.
The packaging gymnastics to support Guzzle 6 + 7 is only needed if PHP 7.1 support is still desired (https://www.php.net/supported-versions.php). If not, this PR can be simplified a lot, using only Guzzle 7 for the test suite and supporting PHP >= 7.2.
To Do