[dependency evaluation] Adopt vincentlanglet/twig-cs-fixer for Twig coding standards

Created on 9 June 2022, about 2 years ago
Updated 12 July 2024, about 17 hours ago

Problem/Motivation

We are using PHP_CodeSniffer for PHP coding standards, ESLint for JavaScript and Stylelint for CSS, but we have nothing for Twig templates.

Steps to reproduce

Proposed resolution

Adopt https://github.com/VincentLanglet/Twig-CS-Fixer for ensuring Twig templates match coding standards.

Dependency evaluation

Maintainership: Primary author is Vincent Langlet, also very active in other projects around Symfony (Symfony itself, Sonata, Doctrine). The author is responsive to issues and pull requests. Only 3 issues are open (2024-07-12), all of them were opened within the last three months.

Security policy: A documented security policy is not available online. https://github.com/VincentLanglet/Twig-CS-Fixer/issues/255 has been opened to address this. Over there, the Vincent Langlet already confirmed that only the latest major version is maintained and receives security updates.

Release cycle: Frequent releases, 19 so far this year. Version 2.0.0 (current major version) has been released at the end of 2023. The maintainer confirmed that he tries to adhere to semver.

Other dependencies: None that aren't already required by Drupal itself or other dependencies.

Remaining tasks

  • Add TwigCS to composer.json and commit-code-check.sh
  • Configure it to pass without any violations for now
  • Raise child issues to fix individual violations

User interface changes

API changes

Data model changes

Release notes snippet

📌 Task
Status

Needs review

Version

11.0 🔥

Component
Theme 

Last updated about 16 hours ago

Created by

🇬🇧United Kingdom longwave UK

Live updates comments and jobs are added and updated live.
  • Coding standards

    It involves compliance with, or the content of coding standards. Requires broad community agreement.

  • Needs framework manager review

    It is used to alert the framework manager core committer(s) that an issue significantly impacts (or has the potential to impact) multiple subsystems or represents a significant change or addition in architecture or public APIs, and their signoff is needed (see the governance policy draft for more information). If an issue significantly impacts only one subsystem, use Needs subsystem maintainer review instead, and make sure the issue component is set to the correct subsystem.

  • Needs release manager review

    It is used to alert the release manager core committer(s) that an issue significantly affects the overall technical debt or release timeline of Drupal, and their signoff is needed. See the governance policy draft for more information.

  • Needs frontend framework manager review

    Used to alert the fron-tend framework manager core committer(s) that a front-end focused issue significantly impacts (or has the potential to impact) multiple subsystems or represents a significant change or addition in architecture or public APIs, and their signoff is needed (see the governance policy for more information). If an issue significantly impacts only one subsystem, use Needs subsystem maintainer review instead, and make sure the issue component is set to the correct subsystem.

Sign in to follow issues

Merge Requests

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • First commit to issue fork.
  • Merge request !5248Twig Coding Standard Rules → (Open) created by saidatom
  • Pipeline finished with Failed
    8 months ago
    Total: 185s
    #44150
  • Pipeline finished with Failed
    8 months ago
    #44160
  • Pipeline finished with Failed
    8 months ago
    #44161
  • Pipeline finished with Failed
    8 months ago
    Total: 154s
    #44164
  • Status changed to Needs review 8 months ago
  • 🇵🇹Portugal saidatom Lisbon

    Twig Coding Standard (twig-cs-fixer) found 120 errors, check attachment.

  • Pipeline finished with Failed
    8 months ago
    #44329
  • Pipeline finished with Failed
    8 months ago
    Total: 155s
    #44330
  • Status changed to Needs work 8 months ago
  • The Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

    This does not mean that the patch needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

    Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

  • Status changed to Needs review 8 months ago
  • 🇵🇹Portugal saidatom Lisbon
  • Pipeline finished with Failed
    8 months ago
    Total: 617s
    #47348
  • Status changed to Needs work 8 months ago
  • The Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

    This does not mean that the patch needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

    Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

  • 🇵🇱Poland wotnak

    MR !5248 adds vincentlanglet/twig-cs-fixer instead of friendsoftwig/twigcs about which is this issue.

    That being said, friendsoftwig/twigcs maintainer is looking for someone to take over the maintainership and considers putting the project in the EOL status if no one will step up.
    https://github.com/friendsoftwig/twigcs/issues/304

    One alternative they point to is vincentlanglet/twig-cs-fixer (which MR !5248 adds).
    Personally, I also prefer vincentlanglet/twig-cs-fixer since it also includes automatic fixes. We already using it for some time at work in Drupal based projects, and it works great.

  • 🇦🇺Australia alex.skrypnyk Melbourne

    Looks like `friendsoftwig/twigcs` will be EOL soon and `vincentlanglet/twig-cs-fixer` is a promising replacement.

    Should we change the title and description to be more generic?

  • 🇪🇸Spain rodrigoaguilera Barcelona

    Yes, vincentlanglet/twig-cs-fixer is a tool with more adoption, more maintained and is more aligned with drupal releases in aspects like phpstan and following symfony releases.

    I don't see the failure in the current PR but anyway it needs a rebase.

  • Pipeline finished with Canceled
    25 days ago
    Total: 71s
    #202084
  • Pipeline finished with Failed
    25 days ago
    Total: 178s
    #202086
  • Status changed to Needs review 24 days ago
  • 🇵🇹Portugal saidatom Lisbon
  • Pipeline finished with Failed
    16 days ago
    Total: 530s
    #209717
  • Status changed to Needs work 8 days ago
  • The Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

    This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

    Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

  • Pipeline finished with Failed
    3 days ago
    Total: 815s
    #220555
  • Pipeline finished with Failed
    3 days ago
    Total: 450s
    #220605
  • Pipeline finished with Success
    3 days ago
    Total: 447s
    #220820
  • Status changed to Needs review 3 days ago
  • 🇵🇹Portugal saidatom Lisbon
  • Status changed to Needs work 2 days ago
  • The Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

    This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

    Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

  • 🇺🇸United States xjm

    #2 is still not addressed; this issue could be a whole lot of wasted effort if the dependency doesn't pass the dependency evaluation. I recommend ceasing rerolling/rebasing/etc. and instead completing the dependency evaluation, then tagging it "Needs release manager review" and "Needs frontend framework manager review".

  • 🇺🇸United States xjm

    Also things with lockfile changes are never good candidates for the friendly bot as they break constantly, and it's not worth rerolling something forever without other progress. So tagging so the bot leaves it alone. Thanks!

  • 🇩🇪Germany Feuerwagen Bonn 🇩🇪🇪🇺

    Took an initial stab at the dependency evaluation. Only thing that seems to be missing is the security policy – opened https://github.com/VincentLanglet/Twig-CS-Fixer/issues/255

  • 🇩🇪Germany Feuerwagen Bonn 🇩🇪🇪🇺
  • Status changed to Needs review about 19 hours ago
  • 🇺🇸United States xjm

    Very nice work @Feuerwagen! Setting NR for review.

  • 🇬🇧United Kingdom longwave UK

    Thanks @Feuerwagen, as the maintainer is very responsive let's wait a few days for the security policy to hopefully be adopted and then we can move forward here.

    Also fixing obsolete link in the IS.

Production build 0.69.0 2024