-
Notifications
You must be signed in to change notification settings - Fork 356
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
[RFR] Add PHP Coding Standard check step on CI. #617
Conversation
'gid' => 1, | ||
'rdev' => 0, | ||
'size' => 666, | ||
'uid' => 123, |
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.
IMHO would be better to keep things aligned here
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.
Actually I'm not fan of aligning things, as it will make changes on other lines as soon as a big line is introduced ; and thus, making a new commit for these lines, whereas they were not significantly changed.
To me, it's the same issue than the trailing comma at the end of array items. Before if was supported, adding a new item to a list was producing a git diff of two lines : the actual line and the above one to add a comma. Only the actual line had a sense for the git diff.
That's why I prefer not to keep things aligned. Only alphabetic sort should improve readability IMHO.
MetadataSupporter | ||
class AclAwareAmazonS3 implements | ||
Adapter, | ||
MetadataSupporter |
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.
Maybe just put everything on a single line? 🤔
What about using PedroTroller/PhpCSFixer-Custom-Fixers and keep a single php_cs config file? |
Only run the
php-cs-compare
make target on travis ASAP to fail the build when the CS are violated.Also, expose a
php-cs-fix
make target to fix the violations.