Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork5.3k
Update book to comply with best practices, round 3#4779
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
wouterj commentedJan 6, 2015
| Q | A |
|---|---|
| Doc fix? | yes |
| New docs? | yes |
| Applies to | all |
| Fixed tickets | #4431 |
best_practices/templates.rst Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
@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 fromcreatePostAction tocreate_post.html.twig could be buggy.
What do guys think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
In this moment, my thinking is that theold way seems the best, as it's very simple: everything just matches: no transformations (of course, you can do whatever you want).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
ping@javiereguiluz I need your input on upper/lower case template directories here - this will affect a lot of different parts. Thanks :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
ping again@javiereguiluz!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
moved these changes to#5001
wouterj commentedJan 6, 2015
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 commentedJan 6, 2015
This will conflict with#4740 |
wouterj commentedJan 6, 2015
I don't think so, as Javier updated the cookbook and I the book |
stof commentedJan 6, 2015
ah, I forgot the fact that his PR does not do the search and replace on the whole doc. 👍 |
book/validation.rst Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
This is not correct, see#4789 (including some other fixes)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
What's the issue? Just that we should have the line breaks different?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
missingnew
book/http_cache.rst Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
This should be something likedefault/homepage.html.twig
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
i would prefer valid php here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
this is our standard
wouterj commentedMar 10, 2015
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 |#4431Commits-------3ab53a6 Fixed some of the commentsc6ff013 Made http cache chapter best-practices-compatible and lots of other fixes4840768 Made propel chapter best-practices-compatible and lots of other fixesa272f99 Made testing chapter best-practices-compatible and lots of other fixesc49f75d Made validation chapter best-practices-compatible
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
very nice
weaverryan commentedMar 13, 2015
* 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.rstbook/testing.rstcookbook/logging/monolog.rstreference/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 ...