-
-
Notifications
You must be signed in to change notification settings - Fork 32
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 Application to support PHP 8.4 #218
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #218 +/- ##
=========================================
Coverage 95.40% 95.40%
Complexity 80 80
=========================================
Files 9 9
Lines 261 261
=========================================
Hits 249 249
Misses 12 12 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add line to changelog.
Better would be catch this via PHP CS Fixer, PHPStan or Rector rule.
Agree. If exist such rule for Rector, you can add it.
rector.php
Outdated
->withPhpSets(php80: true) | ||
->withPhpVersion(PhpVersion::PHP_84); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Little bit strange and unexpected configuration but this is suggested by @samsonasik in:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if you have files applied, better run once, and rollback the config with php version 8.0, so that will be safer, ensuring in case of some rule have multiple php version check.
I think php-cs-fixer or phpcs have this already so you don't need to use it if php version is not in 8.4 yet, see https://cs.symfony.com/doc/rules/function_notation/nullable_type_declaration_for_default_null_value.html
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@samsonasik thx for the feedback and support. Did rollback the config. If the yii core team want to add another tool I let them decide currently there is no php cs tool in place or I'm not aware of it.
I think we can add PHP 8.4 to GitHub CI.https://github.com/yiisoft/yii-console/blob/master/.github/workflows/build.yml |
Ah, roave plugin :( @vjik do we have solution for it? |
@samdark sadly no solution as it is blocked by PHP 8.4 support of Psalm it uses classes from there. |
@alexander-schranz |
I'm not aware how to add such flags via the shared CI actions, without effecting the other PHP versions. So think its better one of the yii core devs take this PR over. |
Let's remove 8.4 from CI for now. Thanks for the pull request. Merged. |
Update Application to support PHP 8.4. Better would be catch this via PHP CS Fixer, PHPStan or Rector rule.