8000 php-cs-fixer/shim is userland · Issue #1644 · symfony/maker-bundle · GitHub
[go: up one dir, main page]
Skip to content

php-cs-fixer/shim is userland #1644

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.

Sign up for GitHub

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

Jump to bottom
Closed
Chris53897 opened this issue Jan 14, 2025 · 4 comments
Closed

php-cs-fixer/shim is userland #1644

Chris53897 opened this issue Jan 14, 2025 · 4 comments

Comments

@Chris53897
Copy link

is it intended that php-cs-fixer/shim is in require and not in require-dev?
4735ff1#diff-d2ab9925cad7eac58e0ff4cc0d251a937ecf49e4b6bf57f8b95aab76648a9d34R19

Now friendsofphp/php-cs-fixer will be uninstalled and php-cs-fixer/shim will be installed in userland code.

@theofidry @jrushlow
@Chris53897 Chris53897 mentioned this issue Jan 14, 2025
Closed
2 tasks
@theofidry
Copy link
Contributor
theofidry commented Jan 14, 2025 •
edited
Loading

Now friendsofphp/php-cs-fixer will be uninstalled and php-cs-fixer/shim will be installed in userland code.

You'll need to expend on why this is the case, it is not the expected behaviour for sure.

From this (symfony/maker-bundle) perspective, the shim is a required dependency: it relies on it to execute it against generated code (so it uses it as part of its source code). However, you likely consume this library as a dev dependency so the shim would remain a dev dependency from your application.

@Chris53897
Copy link
Author
Chris53897 commented Jan 14, 2025 •
edited
Loading

I hope this infos will help.
Yes it is still a dev-dependency.
But there are some little DX issues. PHP-CS-Fixer/PHP-CS-Fixer#8372 (comment)

my composer.json

"friendsofphp/php-cs-fixer": "^3.68",
"symfony/maker-bundle": "^1.62",

symfony composer update

`Symfony operations: 4 recipes (e3eb63c1ad767ffc8e3fff791285e9d0)

  • Configuring php-cs-fixer/shim (>=3.0): From github.com/symfony/recipes:main
  • Unconfiguring friendsofphp/php-cs-fixer (>=3.0): From github.com/symfony/recipes:main
  • Unconfiguring composer/xdebug-handler (>=3.0.5): From auto-generated recipe
  • Unconfiguring composer/pcre (>=3.3.2): From auto-generated recipe
    `

@Seb33300
Copy link

Adding php-cs-fixer/shim as a required dependency trigger the php-cs-fixer/shim recipe in our project:
https://github.com/symfony/recipes/tree/main/php-cs-fixer/shim

@Chris53897 Chris53897 mentioned this issue Jan 14, 2025
Merged
@mbaschnitzi mbaschnitzi mentioned this issue Jan 14, 2025
Closed
@kbond kbond mentioned this issue Jan 15, 2025
Merged
kbond added a commit that referenced this issue Jan 15, 2025
@kbond
bug #1654 Revert #1575 (Use a PHP-CS-Fixer shim rather than an extern…
468ff27 
…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 as completed Jan 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@kbond @Seb33300 @theofidry @Chris53897
0