PHP 8 Support

[driesvints]

Q	A
Bug fix?	no
New feature?	yes
BC breaks?	no
Related Issue	Fix #...
Need Doc update	no

Describe your change

This PR will provide PHP 8 Support for the library. Atm this is blocking laravel/scout#425

I'll keep the PR in draft until I've fixed the build.

[julienbourdeau]

Thanks Dries!
Was it enough to run it on PHP 8 locally?

[driesvints]

@julienbourdeau I'll need to verify that on thursday

[driesvints]

@julienbourdeau I wasn't able to get things running on PHP 8 I'm afraid. I think it's best that we get rid of the PHP CS Fixer dependency as soon as you can. Or at least find a way how we can add --ignore-platform-req=php to the nightly build.

[julienbourdeau]

I think removing php-cs-fixer in the deps should be fine. It's only needed in the CI anyway

[driesvints]

I'm gonna wait a bit with proceeding here until the CircleCI PR has been merged so I can rebase on it.

[chloelbn]

@driesvints circleCI has been merged on master! you can keep going. Thanks for your work!

[driesvints]

Seems like builds for forked PRs aren't enabled yet: https://circleci.com/docs/2.0/oss/#build-pull-requests-from-forked-repositories

[chloelbn]

@driesvints Nice catch thanks! it should be good now

[julienbourdeau]

PHP 8.0 doesn't seem available on circle-ci yet: Error response from daemon: manifest for circleci/php:8.0 not found.

Our tests were written for PHPUnt 4 and won't work with PHPUnit 9 unfortunately.

[chloelbn]

@julienbourdeau luckily for us, as implementing the CTS is in our pipeline we'll be able to upgrade PHPUnit as well too, right?

[julienbourdeau]

@chloelbn Unfortunately, PHPUnit 9 is only compatible with recent php version. And since there are breaking change, you can't have one test suite that works in old PHPUnit and newer PHPUnit 😬
Not sure how we're going to deal with it yet 😩

[driesvints]

@julienbourdeau pushed a change to name the builds. They now appear much more readable here in the GitHub checks overview.

We'll need to provide a matrix for the PHPUnit versions.

[driesvints]

Hey @chloelbn. I've reverted the changes to the safeName method but the builds still seems to fail: https://app.circleci.com/pipelines/github/algolia/algoliasearch-client-php/97/workflows/1e1a9d2e-c7b8-40e3-85bd-a50edc66b250/jobs/460

[thadbryson]

PHP-CS-Fixer added PHP 8 support in version 2.17.0 so that shouldn't be a blocker anymore.

https://github.com/FriendsOfPHP/PHP-CS-Fixer/blob/2.17/composer.json

[thadbryson]

I added my own pull request to get unit tests to work. I bumped PHPUnit to ^9.3.

[driesvints]

Seems like PHP CS Fixer isn't ready yet: https://app.circleci.com/pipelines/github/algolia/algoliasearch-client-php/101/workflows/68475022-554b-4364-b631-811f39e5f9cb/jobs/507/parallel-runs/0/steps/0-109

@thadbryson: PHP-CS-Fixer/PHP-CS-Fixer#5324 (comment)

[driesvints]

@chloelbn I've tried implementing the script but now the tests are failing on PHP because the old state of the tests don't work on newer PHPUnit versions anymore: https://app.circleci.com/pipelines/github/algolia/algoliasearch-client-php/104/workflows/2f529bfb-a235-402b-afb3-59454828a064/jobs/531

https://github.com/Yoast/PHPUnit-Polyfills seems like an alternative but requires the entire test suite to be rewritten as PHPUnit 9 tests.

At this point I'd suggest to create a separate 2.x branch from master which contains the current v2 releases and do a new v3 major version on master (or separate 3.x branch) which drops everything below PHP 7.2. It's the most viable choice because supporting all these PHP versions in a single branch just isn't feasible or maintainable.

[thadbryson]

@chloelbn I've tried implementing the script but now the tests are failing on PHP because the old state of the tests don't work on newer PHPUnit versions anymore: https://app.circleci.com/pipelines/github/algolia/algoliasearch-client-php/104/workflows/2f529bfb-a235-402b-afb3-59454828a064/jobs/531

https://github.com/Yoast/PHPUnit-Polyfills seems like an alternative but requires the entire test suite to be rewritten as PHPUnit 9 tests.

At this point I'd suggest to create a separate 2.x branch from master which contains the current v2 releases and do a new v3 major version on master (or separate 3.x branch) which drops everything below PHP 7.2. It's the most viable choice because supporting all these PHP versions in a single branch just isn't feasible or maintainable.

I agree. They'll have to drop support for PHP 5 and stick with 7.1 and higher. Maintaining both is not feasible.

[asivaneswaran]

Just wondering if there was any ETA on this?

This is kinda blocking me to move my project to 8.0.

Thanks

[lahivee]

It really isn't a plus for Algolia to fail keeping their repositories up2date even 3 weeks (!) after the latest PHP release.

[thadbryson]

I encourage everyone who wants Algolia to bump support to PHP 8.0 to contact them.

https://www.algolia.com/support/

Currently their documention says they support everything at and above 5.3 which is no longer accurate.

[chloelbn]

@thadbryson please do not reach out for support, we have to be mindful of their time while the dedicated API client team is already actively working on this matter. We'll soon be able to merge this PR from @julienbourdeau.

Indeed a new major is required and we're preparing it as we speak, but to make all the changes required it won't be ready before a few weeks. We'll deliver a quick fix in the meantime through the previous PR so that our client won't be in your way anymore to upgrade to PHP 8.

Thanks for your patience and your understanding 💖

[chloelbn]

PHP 8 support has been added to the latest patch version, 2.7.3 🎉
I'm closing this PR, thanks a lot @driesvints and everyone else for your work! A new major version is in the pipeline, but for the moment we hope that this new version will help you move forward in your own projects! Please let us know if you encounter any issues.

Cheers!

[driesvints]

Thanks @chloelbn & @julienbourdeau 👍

[asivaneswaran]

Great news, thanks everyone involved!

@chloelbn @driesvints @julienbourdeau

[arcticlinux]

Thanks everyone!

…

On Tue., Dec. 22, 2020, 7:01 a.m. asivaneswaran, ***@***.***> wrote: Great news, thanks everyone involved! @chloelbn <https://github.com/chloelbn> @driesvints <https://github.com/driesvints> @julienbourdeau <https://github.com/julienbourdeau> — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#644 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAXOLZAKBAW7EJERXBQAGTSWCRDTANCNFSM4TIVH3LA> .