Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork5.3k
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
book/controller.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.
You have to remove the underscore after thedoc role.
wouterj commentedNov 7, 2014
+1, I was about doing the same in my PR :) |
book/controller.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 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!
wouterj commentedNov 20, 2014
let's postpone this to post-2.6 :) |
weaverryan commentedNov 20, 2014
@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! |
wouterj commentedNov 20, 2014
@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.
weaverryan commentedNov 24, 2014
@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! |
wouterj commentedNov 24, 2014
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. |
weaverryan commentedNov 24, 2014
@wouterj Good plan - I think we have thismost right. I'll get the tests passing and get this guy merged in. Thanks :) |
book/controller.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.
You are missing the underscore here (web_server instead ofwebserver).
weaverryan commentedNov 25, 2014
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!