Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork5.3k
Comply with best practices, Round 2#4507
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
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.
why removing this ? form_row indeed renders the errors of the specific field too
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 misread the code example, added it back now
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 The official recommendation is to lowercase the directories, correct? That would mean that@Template won't work (even if it were extended to be able to look in theapp/Resources/views dir), but that certainly doesn't bother me (and I like the consistency of having lowercased resources). I'm just triple-checking before we make a lot of changes.
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.
lowercase? Then I've done it wrong in the previous PR too, we'll need to do that in a new PR then.
weaverryan commentedNov 25, 2014
@wouterj I added some notes, but it looks great. The only pending question is for Javier - but I'm pretty positive we need to change the template directories to lower-case. Thanks for this! I love that you've been taking this on, and I love having an extra set of eyes on these old (as in, originally written years ago) docs. Cheers! |
wouterj commentedNov 25, 2014
Thanks@weaverryan and@stof! I've fixed the remaining comments |
weaverryan commentedDec 7, 2014
Thanks for your hard work on this Wouter! The trick now is that we need to be extra-vigilant for bugs in the near-term as we change so many little things. The more people that can read through the docs and find any bugs, the better :). Cheers! |
This PR was merged into the 2.3 branch.Discussion----------Comply with best practices, Round 2| Q | A| --- | ---| Doc fix? | yes| New docs? | no| Applies to | all| Fixed tickets | partly#4431Less other fixes, more find/replace this time to speed up the process. I have one question this time:* Should we just mention the use of the param converter (the current case), or update all simple controller examples to the use param converter?Commits-------5ee9791 Some fixesfdc460d Minor standard fix for best practices guide2a97453 Minor tweaka29f9fb Don't use form() helper4ef1ef3 Apply best practices to formsde4fcf4 Use AppBundle instead of AcmeStoreBundlec8ce507 Other minor fixes3c71b6d Use AppBundle instead of AcmeDemoBundle
Less other fixes, more find/replace this time to speed up the process. I have one question this time: