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

Best practices template names #5001

Merged
merged 1 commit into from
Mar 13, 2015

Conversation

wouterj
Copy link
Member

@wouterj wouterj commented Feb 16, 2015

@javiereguiluz @weaverryan wanted your opinion on this.

Conflicts:
	book/http_cache.rst
	book/validation.rst
@javiereguiluz
Copy link
Member

My vote goes for the changes that you made on this PR:

// I don't like this syntax
app/Resources/views/Article/articleDetails.html.twig

// this looks nice to me
app/Resources/views/article/article_details.html.twig

My reasons:

  • Article/articleDetails is a bit harder to write than article/article_details because of the mixing of capital letters.
  • Article looks like a class name, but article looks like a directory name.
  • articleDetails looks like a method name, but article_details looks like a template name.

@Koc
Copy link
Contributor

Koc commented Feb 17, 2015

-1. It much easier to correspond controller class -> directory, action method -> template with old naming.

@javiereguiluz
Copy link
Member

@Koc you are right of course. But keep in mind that it's not uncommon for controllers to render templates that don't match 1:1 their class+method name. For example:

class FooController extends Controller
{
    public function barAction()
    {
        // ...

        return $this->render('common/security/login_form.html.twig');

        // ...

        return $this->render('foo/bar.html.twig');
    }
}

In my opinion, the common/... template path looks better than Common/Security/loginForm.html.twig.

@weaverryan
Copy link
Member

👍 for me (though barely). And this kills @Template, as you'd need to break the best practice or update @Template to work with lower-casing (which will be error prone, e.g. ABCDefaultController -> a_b_c_default). @Koc is also right that it's less obvious which controller class the templates belong to.

But I like it because it highlights more clearly that you can name the directory/template whatever you want. I think sometimes that detail is lost to beginners - they're just always saying Default because I tell them to.

What do @symfony/deciders think?

@stof
Copy link
Member

stof commented Mar 2, 2015

@weaverryan the best practice of putting templates in app rather than in the bundle folder is already killing @Template() (it is still possible to use @Template('foo.twig')

@weaverryan
Copy link
Member

@stof Good point. I personally don't really care for @Template.

@wouterj
Copy link
Member Author

wouterj commented Mar 2, 2015

@weaverryan please note that there is a best practice recommending against using @Template:

Don't use the @Template() annotation to configure the template used by the controller.
-- http://symfony.com/doc/current/best_practices/controllers.html

@weaverryan
Copy link
Member

Thanks for this Wouter! Since we heard more agreement than disagreement, we'll recommend this as the standard (of course, people can do whatever they like). Cheers!

@weaverryan weaverryan merged commit 2b65304 into symfony:2.3 Mar 13, 2015
weaverryan added a commit that referenced this pull request Mar 13, 2015
This PR was merged into the 2.3 branch.

Discussion
----------

Best practices template names

@javiereguiluz @weaverryan wanted your opinion on this.

Commits
-------

2b65304 Use snake_case for template names
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.

5 participants