Skip to content

[RFC] use singular folder pattern like other components codes #2743

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

Open
wants to merge 1 commit into
base: 2.x
Choose a base branch
from

Conversation

94noni
Copy link
Contributor

@94noni 94noni commented May 16, 2025

Q A
Bug fix? no
New feature? no
Docs? no
Issues Fix coherence pattern naming
License MIT

opening this PR mostly as RFC
other code in sf document to create special code (messenger message, event listener|subscriber, etc) inside folders in singular form not plural
does It makes sens for twig component to follow this pattern ?

@carsonbot carsonbot added RFC Status: Needs Review Needs to be reviewed labels May 16, 2025
@@ -23,7 +23,7 @@ And (2) a template:

.. code-block:: html+twig

{# templates/components/Alert.html.twig #}
{# templates/component/Alert.html.twig #}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

templates has a "s" marking plural, similarely here to me it should stay plural

@@ -7,7 +7,7 @@ making it easier to render and re-use small template "units" - like an

Every component consists of (1) a class::

// src/Twig/Components/Alert.php
// src/Twig/Component/Alert.php
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// src/Twig/Component/Alert.php
// src/Twig/Component/Alert.php

Something we will discuss in 3.0 .. but i totally agree.

To be fair, i even think we should use src/Component

Copy link
Member

@smnandre smnandre left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 for the namespace (but not now)

👎 for the template folder

@94noni
Copy link
Contributor Author

94noni commented May 17, 2025

top for php file and ok for twig file makes sens
will rewrite this PR on the php codebase side 👍🏻

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants