Skip to content
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

Install & run PHP-CS-Fixer #2250

Merged
merged 6 commits into from Apr 4, 2024
Merged

Conversation

DominicLuidold
Copy link
Contributor

@DominicLuidold DominicLuidold commented Apr 1, 2024

Q A
Bug fix? no
New feature? no
Deprecations? no
Issues /

As there are currently efforts to introduce PHPStan (see #2249), I thought of also introducing PHP-CS-Fixer as a dev dependency (to additionally improve the code quality) and using/upgrading the existing config file.

Already done:

  • Introduce PHP-CS-Fixer as a dev dependency
  • Upgrade the existing config
  • Run PHP-CS-Fixer on the project files

Possible further steps as part of this PR that need to be discussed:

  • Replace StyleCI with a PHP-CS-Fixer job as part of the GitHub Actions CI/continuous-integration workflow (to have a single source of truth regarding code style)?
    • Run PHP-CS-Fixer on same test matrix as PHPUnit1 or just on latest supported PHP version (e.g. 8.3)?

Footnotes

  1. This approach will not work on PHP 7.4 and PHP 8.0 as some files already use newer syntax leading to failures in these jobs.

.php-cs-fixer.dist.php Show resolved Hide resolved
.php-cs-fixer.dist.php Outdated Show resolved Hide resolved
@DjordyKoert
Copy link
Collaborator

DjordyKoert commented Apr 2, 2024

Possible further steps as part of this PR that need to be discussed:

  • Replace StyleCI with a PHP-CS-Fixer job as part of the GitHub Actions CI/continuous-integration workflow (to have a single source of truth regarding code style)?

I also think that we should remove StyleCI in favor of a PHP-CS-Fixer job. I would have to message/email @GuilhemN though asking if he can disable StyleCI for this project before merging this. As far as I can see there is no easy way to disable it using a simple config change in .styleci.yml

  • Run PHP-CS-Fixer on same test matrix as PHPUnit1 or just on latest supported PHP version (e.g. 8.3)?

I think running it on the latest PHP version (8.3) will suffice.

@DominicLuidold DominicLuidold marked this pull request as ready for review April 3, 2024 22:01
@DominicLuidold
Copy link
Contributor Author

I've refactored the current CI workflow a bit and introduced a composite action that allows to re-use the steps for setting up and installing the Composer dependencies across multiple jobs.

This change should make it quite easy to add a PHPStan CI job in #2249 as well 😄

@DjordyKoert
Copy link
Collaborator

Update: I have sent an email to @GuilhemN asking to disable StyleCI for this repo

@DjordyKoert
Copy link
Collaborator

StyleCI has now been disabled, I will apply my own suggestion and merge it asap

@DjordyKoert DjordyKoert merged commit f594e60 into nelmio:master Apr 4, 2024
11 checks passed
@DjordyKoert
Copy link
Collaborator

Thank you very much @DominicLuidold for your work ❤️

@DominicLuidold DominicLuidold deleted the php-cs-fixer branch April 4, 2024 09:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants