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

Run tests for PHP 8 #1199

Closed
wants to merge 1 commit into from
Closed

Run tests for PHP 8 #1199

wants to merge 1 commit into from

Conversation

hivokas
Copy link

@hivokas hivokas commented Nov 5, 2021

No description provided.

@CLAassistant
Copy link

CLAassistant commented Nov 5, 2021

CLA assistant check
All committers have signed the CLA.

@hivokas
Copy link
Author

hivokas commented Nov 8, 2021

Well, I suppose it's not that easy because the codebase doesn't support PHP 8 :(

@remi-stripe
Copy link
Contributor

@hivokas Correct it's unfortunately not easy to add support for this. All the tools we use are either locked on older versions or require dropping older versions of PHP to support PHP 8. While it's something we really want to offer, it'd mean dropped PHP 5.6 and PHP 7.0 which are unfortunately still widely used by developers using Stripe today.

@hivokas
Copy link
Author

hivokas commented Nov 9, 2021

@remi-stripe I've found this branch in this repo: 84c077c.

I believe test cases can be updated to be supported both by PHP 5.6 and PHP 8, both by PHPUnit 5.7 and PHPUnit 9 by not using asserts that are missing either in PHPUnit 5.7 or PHPUnit 9. Then, composer.json can be updated to use PHPUnit ^5.7|^9.0 which will allow GitHub Actions CI workflow to use PHPUnit 5.7 for older PHP versions, PHPUnit 9 for PHP 8. Does it make sense?

@remi-stripe
Copy link
Contributor

cc @richardm-stripe can you have a look at the PR and see whether this is a viable path forward?

@driesvints
Copy link

@remi-stripe I still think that just branching to a new major version is the best way to go to modernize the SDK and drop a bunch of old PHP versions. You can simultaneously support the new major version and the old one so nobody gets left behind.

@lucasmichot
Copy link

@richardm-stripe
Copy link
Contributor

This was subsumed by #1209, which began running CI for both PHP8 and PHP8.1.

@hivokas
Copy link
Author

hivokas commented Dec 2, 2021

This is great @richardm-stripe! Thanks!

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.

6 participants