-
-
Notifications
You must be signed in to change notification settings - Fork 364
[CI] Re-add PHP-CS-Fixer #2803
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
[CI] Re-add PHP-CS-Fixer #2803
Conversation
12cabbe
to
e0d0f06
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.
This works for me. I don't think fabbot ever worked for the website code.
Once merged, I'll disable the webhook?
Yeah no rush to remove it. |
100% 👍 ... and i'm thinking we should either find a way to exclude false positives in fabbot... or find a way to adapt our code to get green results when it runs |
- uses: actions/checkout@v4 | ||
- uses: shivammathur/setup-php@v2 | ||
with: | ||
php-version: '8.1' |
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.
we could use the php-cs-fixer tool provided by shivammathur also
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.
But we won't be able to run it locally, see #617
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.
Yep, easier to use the installed version so you don't have to duplicate version constraints. I forgot that I don't include in my packages - the ci fixes automatically.
php-version: '8.1' | ||
- run: composer install | ||
- name: php-cs-fixer | ||
run: ./vendor/bin/php-cs-fixer fix --dry-run --diff |
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.
run: ./vendor/bin/php-cs-fixer fix --dry-run --diff | |
run: ./vendor/bin/php-cs-fixer fix --dry-run --diff --show-progress=none |
I have a doubt to be honest, it may be automatically set
e0d0f06
to
1ab48b6
Compare
Follow #2802, #2074, #2014 and more.
I feel like Fabbot only runs on modified files for a given PR, but sometimes it does not really help (e.g.: if you untouched files contain old references to a deleted class).
This behavior is understandable for symfony/symfony, which contains a tons of code and is super active, but that's not the case symfony/ux, we have very less code and less activity.
EDIT: Well, I'm still not sure... PHP-CS-Fixer is showing me fixes for
src/LiveComponent/src/Util/TypeHelper.php
, added in #2778, but it was totally ignored by Fabbot... 😫So, I suggest to re-add PHP-CS-Fixer run in the CI to ensure all files are correctly CS-fixed.