Update the_controller.rst #4644
Conversation
Corrected error in the sample code.
| @@ -205,8 +205,8 @@ different controller using the ``forward()`` method:: | |||
| public function indexAction() | |||
| { | |||
| return $this->forward('AppBundle:Blog:index', array( | |||
| 'name' => $name | |||
| ); | |||
| 'name' => 'Fabien' | |||
There was a problem hiding this comment.
Great fixes! Could you please add a , after this line? Then we're ready.
There was a problem hiding this comment.
A comma after the last array element? Why so?
There was a problem hiding this comment.
it's the Symfony standard. This way, if a new item is added later on, you don't have to add a comma to the previous line (which will mess up the diff in the PR and it's something that's often forgotten).
There was a problem hiding this comment.
Ok, that makes sense.
|
Hmm, while the code does not contain any errors now (which is very nice, thanks!), it still doesn't make sense if you read it (I know this is not your fault, but I just want to point out that we might have to replace it with another example):
I'm not sure if there is a better example (I never understood the real usage of forwarding). |
|
Saying that, it makes me wonder if we should include this in the quick tour at all. :) /cc @javiereguiluz @weaverryan |
|
I think an example about forwarding isn't essential but makes sense. |
|
Hi guys! Thanks for catching the bug :). For me the answer is easy - remove the forwarding section entirely. I think there are likely other changes we need to make (things we need to remove), but for this issue - let's update it to remove the forwarding entirely and then it'll be a great improvement. I asked on Twitter awhile ago about forwarding, and there are almost not use-cases and almost nobody uses them. It's something we should document (and we do), but not in the quick tour :). @teggen do you mind updating your PR to remove this entire section? If you're short on time, let us know and we can help. Cheers! |
|
ping @teggen If you don't have any time soon, let us know and we'll take this over. |
|
I've created a new PR with your commits and with a new commit removing the forwarding section: #5037 Thank you for reporting this issue and trying to fix it! |
This PR was merged into the 2.3 branch. Discussion ---------- Finish 4644: Update the_controller.rst Replaces #4644 | Q | A | --- | --- | Doc fix? | yes | New docs? | no | Applies to | all | Fixed tickets | - Commits ------- dbc25a6 Removed forwarding section from the quick tour 17c9257 Update the_controller.rst 77342fa Update the_controller.rst
Corrected error in the sample code.