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

Address issue with php-cs-fixer #2779

Closed
wants to merge 2 commits into from
Closed

Conversation

bobbrodie
Copy link
Contributor

@bobbrodie bobbrodie commented Dec 26, 2024

Overview

I was checking out this package on my machine and came across an issue -- I noticed that the format script added to composer.json wouldn't run. There were two issues:

  1. The package did not include the friendsofphp/php-cs-fixer development dependency, so there was an expectation that it would be installed globally
  2. No path was set on the format command

Changes

NOTE: To start, I didn't want to make any bad assumptions. Since there was not an existing configuration for php-cs-fixer, I left it alone and only fixed what was already there.

Add dependency

I added the current version of the dependency:

"friendsofphp/php-cs-fixer": "^3.65",

Update Composer script

The script was defined like this:

"format": "php-cs-fixer fix --allow-risky=yes",

But since it needs a path, it's now changed to:

"format": "php-cs-fixer fix --allow-risky=yes .",

Run php-cs-fixer

Finally, I ran composer format and let it run through the project.

Results

The changes are minimal, and all tests are passing. I ran a test without --allow-risky=yes and no additional files are changed, so it might be a good idea to remove this, but again I didn't want to assume why it might be there so I kept this initial pass a straightforward one.

@bobbrodie bobbrodie changed the title Php cs fixer Address issue with php-cs-fixer Dec 26, 2024
@bobbrodie bobbrodie marked this pull request as draft December 26, 2024 06:22
@bobbrodie bobbrodie closed this Dec 26, 2024
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.

1 participant