-
Notifications
You must be signed in to change notification settings - Fork 144
[TASK] Update PHP-CS-Fixer #592
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
Conversation
1d154ec
to
e847cd9
Compare
e847cd9
to
cc1c980
Compare
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.
I just have a query about the comment in the config file, otherwise looks fine.
cc1c980
to
57c88df
Compare
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.
I still think the comment is misleading - unless the defaults have actually changed and the docs are out of date.
It appears the defaults have changed, breaking PHP 7.4 (even though it is supported by the tool), and all CI on whatever PHP version. The latest version thus breaks semantic versioning. (I ran Does the tool claim to follow semantic versioning? If not, then we should have a tilde version contraint in |
57c88df
to
3f94efd
Compare
No, I don't think so.
I'd prefer not to: In |
That PHP-CS-Fixer supports PHP 7.4 means that it will run on PHP 7.4., not that it will create PHP 7.4-compliant code with all possible configurations. (The PER2 standard has some rules that only work on PHP 8.x. We'll need to avoid those for the time being.) |
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.
The comment now accurately conveys the reason for the setting, I think.
Apparently not, with the |
Oh, I wasn't aware of that. So my learning is: Whether PHP-CS-Fixer runs under a specific PHP version also depends on the configuration of the rules. Thanks for pointing that out! 🙏 |
Looking again, it seems that the default for I think this is a bug in PHP-CS-Fixeer. The execution environment is irrelevant. The target environment is the important one. Also, since We should remove the For completeness, here is the error:
|
Since found it's not a BC introduced in PHP-CS-Fixer itself, but a in the |
No description provided.