-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
[PHP CS Fixer] Update the config #1611
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
| ->notPath('public/index.php') | ||
| ->notPath('importmap.php') | ||
| ; | ||
|
|
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 current Finder config first includes the whole project dir and then basically excludes everything but 'src' and 'tests'. This can be simplified to include just 'src' and 'tests'.
| 'no_useless_else' => true, | ||
| 'no_useless_return' => true, | ||
| 'php_unit_strict' => true, | ||
| 'phpdoc_order' => true, |
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.
This rule is also part of the Symfony ruleset but with a different order. To me it makes more sense for the Symfony Demo Application to use the same order as the Symfony project.
| 'phpdoc_order' => true, | ||
| 'strict_comparison' => true, | ||
| 'strict_param' => true, | ||
| 'blank_line_between_import_groups' => false, |
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.
This rule is set to the opposite of the rule that is part of the Symfony ruleset. Again I think it makes more sense to follow the Symfony project.
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 result is pretty ugly in my opinion ... but you are completely right. Following the Symfony practices strictly should be the goal of this repository. Thanks.
|
This is one of those things that often gets neglected as "unimportant", so it never gets updated. But as you've shown here, it actually matters ... for example, to stay aligned with Symfony's best practices. Thanks a lot, Jan, for taking the time to update this! |
This PR simplifies the Finder config and removes the rules that are already part of the Symfony ruleset