Comply with best practices, Round 2 #4507
Conversation
|
|
||
| The majority of the work is done by the ``form_row`` helper, which renders | ||
| the label, errors and HTML form widget of each field inside a ``div`` tag |
There was a problem hiding this comment.
why removing this ? form_row indeed renders the errors of the specific field too
There was a problem hiding this comment.
I misread the code example, added it back now
| @@ -106,7 +96,7 @@ from inside a controller:: | |||
| ->add('save', 'submit', array('label' => 'Create Task')) | |||
| ->getForm(); | |||
|
|
|||
| return $this->render('AcmeTaskBundle:Default:new.html.twig', array( | |||
| return $this->render('Default/new.html.twig', array( | |||
There was a problem hiding this comment.
@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 the app/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.
lowercase? Then I've done it wrong in the previous PR too, we'll need to do that in a new PR then.
|
@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! |
|
Thanks @weaverryan and @stof! I've fixed the remaining comments |
|
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 #4431 Less 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 fixes fdc460d Minor standard fix for best practices guide 2a97453 Minor tweak a29f9fb Don't use form() helper 4ef1ef3 Apply best practices to forms de4fcf4 Use AppBundle instead of AcmeStoreBundle c8ce507 Other minor fixes 3c71b6d Use AppBundle instead of AcmeDemoBundle
Less other fixes, more find/replace this time to speed up the process. I have one question this time: