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

Replace php-cs-fixer/shim with friendsofphp/php-cs-fixer #1651

Closed
wants to merge 1 commit into from

Conversation

mbaschnitzi
Copy link

In #1575 the dependency php-cs-fixer/shim was added to replace a self packaged phar of php-cs-fixer.

This change causes php-cs-fixer/shim to be installed in all projects using symfony/maker-bundle, even if the non-shim package was used before. In symfony flex projects this also causes further unexpected changes since php-cs-fixer/shim also executes an additional recipe modifying additional files. (#1644 #1648)

I have changed the dependency to friendsofphp/php-cs-fixer instead, so the decision on which package to use can be made by the user instead of forcing the use of the shim package. The execution in \Symfony\Bundle\MakerBundle\Util\TemplateLinter remains the same for both packages.

The dependency php-cs-fixer/shim was replaced with
friendsofphp/php-cs-fixer so that the decision over which package to use
is on the user.
@Seb33300
Copy link

friendsofphp/php-cs-fixer also triggers a symfony flex recipe

@mbaschnitzi
Copy link
Author

friendsofphp/php-cs-fixer also triggers a symfony flex recipe

Then maybe the change should be reverted and/or php-cs-fixer should be added as an optional dependency?

If php-cs-fixer is installed globally neither php-cs-fixer/shim or friendsofphp/php-cs-fixer are really needed.

kbond added a commit that referenced this pull request Jan 15, 2025
…al PHAR) (kbond)

This PR was merged into the 1.x-dev branch.

Discussion
----------

Revert #1575 (Use a PHP-CS-Fixer shim rather than an external PHAR)

This reverts #1575. This PR has caused a lot of trouble as it made php-cs-fixer a required dep which installs a recipe.

See #1644, #1653, #1651, #1648.

We can't have php-cs-fixer (or the shim) as a required dependency. I think we need to have this bundle only use php-cs-fixer if available/configured (or drop it entirely). I'm not sure why it is used at all - feels like a lot of added complexity...

Commits
-------

41744d7 Revert "feature #1575 [make:*] Use a PHP-CS-Fixer shim rather than an external PHAR"
@kbond
Copy link
Member

kbond commented Jan 15, 2025

I've reverted #1575 and released v1.62.1. It was causing a lot of problems. I think we need to rethink the php-cs-fixer integration.

@kbond kbond closed this Jan 15, 2025
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.

3 participants