-
Notifications
You must be signed in to change notification settings - Fork 9.4k
Remove old code for php versions we no longer support #39362
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
Comments
Hi @ihor-sviziev. Thank you for your report.
Join Magento Community Engineering Slack and ask your questions in #github channel. |
Hi @engcom-Hotel. Thank you for working on this issue.
|
Hello @ihor-sviziev, Thanks for the report and collaboration! We are confirming this issue for further processing. Thanks |
✅ Jira issue https://jira.corp.adobe.com/browse/AC-13349 is successfully created for this GitHub issue. |
✅ Confirmed by @engcom-Hotel. Thank you for verifying the issue. |
I also noticed that we have some workarounds for bugs in PHP itself: https://github.com/search?q=repo%3Amagento%2Fmagento2%20bugs.php.net&type=code |
@magento I am working on this |
I just happen to have stumbled upon this blog post: https://staabm.github.io/2024/11/14/phpstan-php-version-narrowing.html If I try this out, by changing: --- a/dev/tests/static/testsuite/Magento/Test/Php/_files/phpstan/phpstan.neon
+++ b/dev/tests/static/testsuite/Magento/Test/Php/_files/phpstan/phpstan.neon
@@ -2,13 +2,17 @@ parameters:
checkExplicitMixedMissingReturn: true
checkPhpDocMissingReturn: true
reportUnmatchedIgnoredErrors: false
+ phpVersion:
+ min: 80200
+ max: 80499
excludePaths:
- %rootDir%/../../../lib/internal/Magento/Framework/ObjectManager/Test/Unit/* Update: it turns out this change isn't needed, phpstan extracts the min & max php versions supported from the Then upgrading phpstan to v2 And then running a check on one of the files that still use code for detecting older php versions, phpstan is indeed able to detect automatically code that can be cleaned up:
This requires us to:
I don't see any of this happening soon-ish, Adobe is very slow in updating their static analysis code, so this will probably take a few years until any of this happens, unless they surprise us ... Anyway, leaving this behind for anyone who cares or is interested. |
In #39202, we did some cleanup, but it cleaned not all places.
It would be great to clean also
Originally posted by @ihor-sviziev in #39202 (comment)
The text was updated successfully, but these errors were encountered: