-
Notifications
You must be signed in to change notification settings - Fork 193
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
PHP 8.1 compatibility #489
Conversation
Seems as some packages have to catch up with PHP8.1. |
Looks like the composer package for |
Looks like php-cs-fixer still has problems with 8.1. There are changes to address this in the master branch, but they are not live, yet. I'll temporarily set the php-cs-fixer version to 'dev-master' just to see, if there are more issues. |
Ok, PHP 8.1 support in php-cs-fixer |
Codecov Report
@@ Coverage Diff @@
## main #489 +/- ##
=========================================
Coverage 95.11% 95.11%
Complexity 946 946
=========================================
Files 95 95
Lines 2313 2313
=========================================
Hits 2200 2200
Misses 113 113 Continue to review full report at Codecov.
|
Looks like php-cs-fixer requires up to 8.0, but does not have 8.1 as a requirement: https://github.com/FriendsOfPHP/PHP-CS-Fixer/blob/master/composer.json#L17 I've opened an issue with them in order to get PHP 8.1 support added. |
PHP CS Fixer doesn't yet suport 8.1: PHP-CS-Fixer/PHP-CS-Fixer#5891 We will either have to wait for that or use a different library. |
php-cs-fixer + 8.1 support dropped yesterday (v3.4.0) so this should be good to go, now. |
Getting closer - now it looks like we are waiting on a protobuf fix for 8.1: protocolbuffers/protobuf#9293 |
I've been playing with allowing some jobs to fail and found https://docs.github.com/en/actions/learn-github-actions/workflow-syntax-for-github-actions#example-preventing-a-specific-failing-matrix-job-from-failing-a-workflow-run strategy:
fail-fast: false
matrix:
php-versions: ['7.4', '8.0']
experimental: [false]
include:
- php-versions: 8.1
experimental: true
runs-on: ubuntu-latest
continue-on-error: ${{ matrix.experimental }} |
If we allow the PHP8.1 checks to fail, there is no much value in running them in the first place imo, or maybe I'm just not understanding, what you are trying to achieve by doing this. |
I don't think we need to wait on protobuf to fix things. The is just an indicator for our code being to tightly coupled to the protobuf library, so stuff is leaking into our unit tests. I will address this in a different PR. However there is still a problem with with this line, which lets the unit test run crash. We should discuss in the next meeting, if we want to support such edge cases like context switching inside of a Fiber in the main library by using FFI. I don't think the dev images have even FFI installed, but I have tested this on my local system. I personally like all the stuff one can do with FFI, but it is still marked as experimental. |
It means we can start running our tests against new PHP versions as soon as possible (before they go GA), and find issues earlier. In the case of 8.1, once it does go green, we switch off "experimental" and then it must always pass. |
Ok, got you. let's talk about this in tomorrows meeting, as the FFI issue will not just turn green, the tests are crashing after 51%. |
Related issue: #515 |
PHP 8.1 has been released on Nov. 25th.
So this PR just adds PHP 8.1 to the test matrix.