Update book to comply with best practices, round 3 #4779

wouterj · 2015-01-06T00:41:01Z

Q	A
Doc fix?	yes
New docs?	yes
Applies to	all
Fixed tickets	#4431

wouterj · 2015-01-06T00:42:38Z

best_practices/templates.rst

@@ -51,6 +51,10 @@ Another advantage is that centralizing your templates simplifies the work
 of your designers. They don't need to look for templates in lots of directories
 scattered through lots of bundles.

+.. best-practice::
+
+    Use lowercased snake_case for directory and template names.


I believe this was the guideline, but it was never written somewhere. As lots of people are struggling with it, I think it's a good idea to document it. While doing this, I've also snake_cased all template paths in the currently updated book articles.

@javiereguiluz What do you think? We've never talked too much about this directly. The negative to underscoring everything is that templates aren't the same as their controllers anymore - PostAdminController::createPostAction would be in post_admin/create_post.html.twig.

We're just talking about a recommendation here - not life or death - but let's think about it. The old upper-camel case was done without really thinking, to support the @Template magic (because we were all very used to having this from 1.x). If we break from this, then @Template won't work, and even if we update it, trying to transform from createPostAction to create_post.html.twig could be buggy.

What do guys think?

In this moment, my thinking is that the old way seems the best, as it's very simple: everything just matches: no transformations (of course, you can do whatever you want).

ping @javiereguiluz I need your input on upper/lower case template directories here - this will affect a lot of different parts. Thanks :)

ping again @javiereguiluz!

moved these changes to #5001

wouterj · 2015-01-06T00:45:03Z

While changing the Testing and HttpCache articles, I got a bit distracted fixing other things :) I'll do a follow up PR of a Testing article rewrite, as it isn't really nicely written (and it's the oldest not actively maintained document).

stof · 2015-01-06T01:59:45Z

This will conflict with #4740

wouterj · 2015-01-06T07:02:27Z

I don't think so, as Javier updated the cookbook and I the book

stof · 2015-01-06T09:09:57Z

ah, I forgot the fact that his PR does not do the search and replace on the whole doc.

👍

ifdattic · 2015-01-07T11:26:39Z

book/validation.rst

-                $metadata->addPropertyConstraint(
-                    'city',
-                    Length(array("min" => 3)));
+                $metadata->addPropertyConstraint('city', Assert\Length(array(


This is not correct, see #4789 (including some other fixes)

What's the issue? Just that we should have the line breaks different?

missing new

weaverryan · 2015-01-16T22:48:39Z

book/http_cache.rst

-        $response->isNotModified($request);
+        public function homepageAction(Request $request)
+        {
+            $response = $this->render('homepage.html.twig');


This should be something like default/homepage.html.twig

…ixes

timglabisch · 2015-02-18T22:49:16Z

book/http_cache.rst

+            // the ETag or the Last-Modified value
+            // (based on the Request, data is retrieved from
+            // a database or a key-value store for instance)
+            $article = ...;


i would prefer valid php here.

this is our standard

wouterj · 2015-03-10T10:08:02Z

Updated. Thansk for the reviews!

…terJ) This PR was merged into the 2.3 branch. Discussion ---------- Update book to comply with best practices, round 3 | Q | A | --- | --- | Doc fix? | yes | New docs? | yes | Applies to | all | Fixed tickets | #4431 Commits ------- 3ab53a6 Fixed some of the comments c6ff013 Made http cache chapter best-practices-compatible and lots of other fixes 4840768 Made propel chapter best-practices-compatible and lots of other fixes a272f99 Made testing chapter best-practices-compatible and lots of other fixes c49f75d Made validation chapter best-practices-compatible

weaverryan · 2015-03-13T00:40:35Z

book/testing.rst

+    $link = $crawler
+        ->filter('a:contains("Greet")') // find all links with the text "Greet"
+        ->eq(1) // select the second link in the list
+        ->link() // and click it


weaverryan · 2015-03-13T00:48:35Z

@wouterj I reviewed this carefully and there is SO much good stuff here. Nice work! I had only very minor changes at sha: 24f773c - especially considering the size of this PR.

Thanks!

* 2.3: (23 commits) Update configuration.rst Update configuration.rst Readability [#4875] Minor language tweaks [#4779] Minor tweaks to a huge PR Fixed some of the comments [#4793] Very minor tweak that sounds more natural [config][translator] use the new fallbacks option. Linked "logrotate" to its official website Minor rewording Added a note about log file sizes and the use of logrotate Use snake_case for template names Made http cache chapter best-practices-compatible and lots of other fixes Made propel chapter best-practices-compatible and lots of other fixes Made testing chapter best-practices-compatible and lots of other fixes Made validation chapter best-practices-compatible Remove usage of first person Updated as per feedback. Updated as per feedback Fix typos ... Conflicts: book/forms.rst book/testing.rst cookbook/logging/monolog.rst reference/configuration/framework.rst

* 2.6: (32 commits) Update configuration.rst Update configuration.rst Readability [#4875] Minor language tweaks [#4779] Minor tweaks to a huge PR [#4724] Minor language tweaks and cross-link to form theming document the validation payload option Fixed some of the comments [#4793] Very minor tweak that sounds more natural [#4732] Tweaking language, clarifying purpose of disabling form and that you can disable CSRF on 1 form [#4732] Tweaking language, clarifying purpose of disabling form and that you can disable CSRF on 1 form [config][translator] use the new fallbacks option. add missing reference options and descriptions Linked "logrotate" to its official website Minor rewording Added a note about log file sizes and the use of logrotate Use snake_case for template names Fixed minor grammar issues Made http cache chapter best-practices-compatible and lots of other fixes Made propel chapter best-practices-compatible and lots of other fixes ...

* 2.7: (38 commits) Update configuration.rst Update configuration.rst Readability [#4875] Minor language tweaks [#4779] Minor tweaks to a huge PR [#4724] Minor language tweaks and cross-link to form theming [Serializer] Fix CS document the validation payload option Fixed some of the comments [#4793] Very minor tweak that sounds more natural [#4732] Tweaking language, clarifying purpose of disabling form and that you can disable CSRF on 1 form [#4732] Tweaking language, clarifying purpose of disabling form and that you can disable CSRF on 1 form [config][translator] use the new fallbacks option. add missing reference options and descriptions Linked "logrotate" to its official website Minor rewording Added a note about log file sizes and the use of logrotate Use snake_case for template names Fixed minor grammar issues Made http cache chapter best-practices-compatible and lots of other fixes ...

wouterj mentioned this pull request Jan 6, 2015

Update book to comply with best practices #4431

Closed

13 tasks

wouterj reviewed Jan 6, 2015
View reviewed changes

wouterj added the Status: Reviewed label Jan 6, 2015

ifdattic reviewed Jan 7, 2015
View reviewed changes

wouterj mentioned this pull request Jan 11, 2015

Update http_cache.rst #4809

Closed

weaverryan reviewed Jan 16, 2015
View reviewed changes

wouterj force-pushed the best_practices_3 branch from 1fb5a7e to 6d10392 Compare January 17, 2015 00:24

weaverryan added Status: Needs Work and removed Status: Reviewed labels Jan 19, 2015

wouterj mentioned this pull request Feb 5, 2015

Twig template naming convention #2515

Closed

wouterj added 4 commits February 16, 2015 17:59

Made validation chapter best-practices-compatible

c49f75d

Made testing chapter best-practices-compatible and lots of other fixes

a272f99

Made propel chapter best-practices-compatible and lots of other fixes

4840768

Made http cache chapter best-practices-compatible and lots of other f…

c6ff013

…ixes

wouterj force-pushed the best_practices_3 branch from 6d10392 to ce95b52 Compare February 16, 2015 21:50

timglabisch reviewed Feb 18, 2015
View reviewed changes

Fixed some of the comments

3ab53a6

wouterj force-pushed the best_practices_3 branch from ce95b52 to 3ab53a6 Compare March 10, 2015 10:07

wouterj added Status: Reviewed and removed Status: Needs Work labels Mar 10, 2015

weaverryan merged commit 3ab53a6 into symfony:2.3 Mar 13, 2015

weaverryan reviewed Mar 13, 2015
View reviewed changes

weaverryan added a commit that referenced this pull request Mar 13, 2015

[#4779] Minor tweaks to a huge PR

24f773c

wouterj deleted the best_practices_3 branch March 17, 2015 08:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update book to comply with best practices, round 3 #4779

Update book to comply with best practices, round 3 #4779

wouterj commented Jan 6, 2015

wouterj Jan 6, 2015

weaverryan Jan 16, 2015

weaverryan Jan 16, 2015

weaverryan Jan 25, 2015

weaverryan Jan 30, 2015

wouterj Feb 16, 2015

wouterj commented Jan 6, 2015

stof commented Jan 6, 2015

wouterj commented Jan 6, 2015

stof commented Jan 6, 2015

ifdattic Jan 7, 2015

weaverryan Jan 16, 2015

wouterj Jan 16, 2015

weaverryan Jan 16, 2015

timglabisch Feb 18, 2015

wouterj Feb 18, 2015

wouterj commented Mar 10, 2015

weaverryan Mar 13, 2015

weaverryan commented Mar 13, 2015

Update book to comply with best practices, round 3 #4779

Update book to comply with best practices, round 3 #4779

Conversation

wouterj commented Jan 6, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wouterj commented Jan 6, 2015

stof commented Jan 6, 2015

wouterj commented Jan 6, 2015

stof commented Jan 6, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wouterj commented Mar 10, 2015

Choose a reason for hiding this comment

weaverryan commented Mar 13, 2015