Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork5.2k
Completely re-reading the controller chapter#4433
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
| class called ``HelloController`` inside a bundle named ``AppBundle``. The | ||
| method ``indexAction()`` is then executed. | ||
| Now, you can go to ``/hello/ryan`` (e.g. ``http://localhost:8000/app_dev.php/hello/ryan`` | ||
| if you're using the :doc:`built-in web server </cookbook/webserver/built_in>`_) |
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.
You have to remove the underscore after thedoc role.
+1, I was about doing the same in my PR :) |
| @@ -762,22 +747,26 @@ headers and content that's sent back to the client:: | |||
| $response = new Response(json_encode(array('name' => $name))); | |||
| $response->headers->set('Content-Type', 'application/json'); | |||
| .. tip:: | |||
| The ``headers`` property is a :class:`Symfony\\Component\\HttpFoundation\\HeaderBag` | |||
| object some nice methods for getting and setting the headers. The header | |||
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 sentence doesn't sound complete.
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.
you're right!
let's postpone this to post-2.6 :) |
@wouterj while not strictly related to 2.6, I'd like to have things like this done as close to 2.6 launch as possible. There is a good amount of work, but I don't want to postpone and drag it on. Now, that would require me finding time of course. So, I'm not saying you're wrong, I'm more saying that Iwant to do this kind of stuff before 2.6 ;). Thanks! |
@weaverryan I predict that I'm not able to review this PR before 2.6, given the amount of work that has more priority (best practices update, missing 2.6 features, sf installer, etc.) That was my reason to say I would like to postpone this :) |
I could find very little real use-cases for this.
@wouterj I understand! Do your best, but of course we're all just giving our free time :). And thanks for the feedback - I've just pushed updates based on everything. The only pending issue I know of is this one:#4433 (comment). Wouter - have you reviewed this entire PR when you originally added comments, or only part of it? Thanks! |
I don't believe I did, although my comments reached from start to end... We don't have to stop this PR from merging though, I can always create a follow up. |
@wouterj Good plan - I think we have thismost right. I'll get the tests passing and get this guy merged in. Thanks :) |
| controller and passes in ``ryan`` for the ``$name`` variable. Creating a | ||
| "page" means simply creating a controller method and associated route. | ||
| Now, you can go to ``/hello/ryan`` (e.g. ``http://localhost:8000/app_dev.php/hello/ryan`` | ||
| if you're using the :doc:`built-in web server </cookbook/webserver/built_in>`) |
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.
You are missing the underscore here (web_server instead ofwebserver).
I've merged this in - thanks for the help! Let's keep moving forward with 2.6 updates and re-reading parts of the docs to have more updates like this. Yes! |
This PR was merged into the 2.3 branch.Discussion----------Completely re-reading the controller chapter| Q | A| --- | ---| Doc fix? | yes| New docs? | no| Applies to | 2.3+| Fixed ticket | n/aHi guys!This follows#4427, but only for the controller chapter (I will open separate PR's for `from_flat_php_to_symfony2`, `routing`, and `templating`).This is more than just updating for the best practices. This is about face-lifting these important chapters after several years of us as a community realizing what's more important and what's less important. More notes:- I removed some information about using the `defaults` to pass additional information to your controller from this chapter. This is too early and we have this now:http://symfony.com/doc/current/cookbook/routing/extra_information.html- I removed `ContainerAware` information. This is not the right spot... and maybe nothing is. I don't see much use-case for this - it seems like you'd either extend the base Controller or register your controller as a service.- I kind of want to remove or move the forwarding stuff. I can't find a real use-case for doing a forward from one controller to another. It adds a lot of overhead and I don't see any situation for it.Thanks!Commits-------903f52c Fixing build errorceb7b94 Big update based on feedback from xabbuh and WouterJ6ef10db Moving forwarding section all the way to the bottom0754efc Completely re-reading the controller chapter
Hi guys!
This follows#4427, but only for the controller chapter (I will open separate PR's for
from_flat_php_to_symfony2,routing, andtemplating).This is more than just updating for the best practices. This is about face-lifting these important chapters after several years of us as a community realizing what's more important and what's less important. More notes:
defaultsto pass additional information to your controller from this chapter. This is too early and we have this now:http://symfony.com/doc/current/cookbook/routing/extra_information.htmlContainerAwareinformation. This is not the right spot... and maybe nothing is. I don't see much use-case for this - it seems like you'd either extend the base Controller or register your controller as a service.Thanks!