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

[RFC] include a new dev dependency twig-cs-fixer in the codebase #54799

Open
94noni opened this issue May 2, 2024 · 9 comments
Open

[RFC] include a new dev dependency twig-cs-fixer in the codebase #54799

94noni opened this issue May 2, 2024 · 9 comments
Labels
RFC RFC = Request For Comments (proposals about features that you want to be discussed)

Comments

@94noni
Copy link
Contributor

94noni commented May 2, 2024

Description

Hello

May I propose to add the tool twig-cs-fixer (repo) in this repo as a dev dependency

RFC

What?

  • this tool is like the php-cs-fixer, but for twig files
  • it is based on official twig coding standard, and allows to lint, and also fix, your twig files

Why?

  • it is listed on twig official documentation
  • it is solid and working great
  • it adds value as code standard, as per all other cs fixers

How?

  • We can add it, starting from 7.2, make it run on symfony/symfony twig files present in the codebase

Thank you!


friendly ping @VincentLanglet and our discussion here

Example

No response

@carsonbot carsonbot added the RFC RFC = Request For Comments (proposals about features that you want to be discussed) label May 2, 2024
@VincentLanglet
Copy link
Contributor

For context it was added by @smnandre in symfony/ux
symfony/ux#1166
symfony/ux#1793

@smnandre
Copy link
Contributor

smnandre commented May 2, 2024

For context it was added by @smnandre in symfony/ux

... only on the website (for now) :)


Regarding the suggestion, did you run it on the Symfony codebase ? Does it change a lot of files ? Only in "test / fixtures" or in Resources also ?

That'd be nice to know the "impact" i think :)

@VincentLanglet
Copy link
Contributor

Regarding the suggestion, did you run it on the Symfony codebase ? Does it change a lot of files ? Only in "test / fixtures" or in Resources also ?

That'd be nice to know the "impact" i think :)

I didn't run it everywhere but I already found some issues like #54051

@derrabus
Copy link
Member

derrabus commented May 2, 2024

May I propose to add the tool twig-cs-fixer (repo) in this repo as a dev dependency

None of the tools we use (PHPUnit, PHP-CS-Fixer, Psalm) is an actual dev dependency on this repo. I don't think we want to change that.

@94noni
Copy link
Contributor Author

94noni commented May 2, 2024

actual dev dependency on this repo

yes I perhaps miswrite my purpose, more like adding a twig-cs-fix config file in the repo as its done for the php-cs-fixer
and from time to time, run it or via CI

did you run it on the Symfony codebase ?

nope, but I can of course and report back here when I 've done it

@94noni
Copy link
Contributor Author

94noni commented May 2, 2024

with a default config file, and the latest version of the fixer
on symfony/symfony 7.1, only src/Bundles
94noni@5f57bab

looks like major things are spacing inside arrays and around operators

@chalasr
Copy link
Member

chalasr commented May 11, 2024

Should it be fabbot's responsibility?
Screenshot 2024-05-11 at 17 09 27

@smnandre
Copy link
Contributor

@chalasr If so, it should probably be Symfony specific then (at least at first) ?

@chalasr
Copy link
Member

chalasr commented May 11, 2024

@smnandre Perhaps yes, I don't know much about fabbot internals as it's not public code yet. I'd expect it to run tools like php-cs-fixer only if a matching config file exists in the project. I may be wrong though.
What I'm pretty sure is, if we check twig files as part of the CI, only the files that changed in the given PR/commit should be checked (which is how fabbot operates).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
RFC RFC = Request For Comments (proposals about features that you want to be discussed)
Projects
None yet
Development

No branches or pull requests

6 participants