-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
[WIP] Use controller autowiring / DunglasActionBundle #315
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@@ -1,18 +1,13 @@ | |||
services: | |||
# First we define some basic services to make these utilities available in | |||
# the entire application | |||
slugger: |
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 is the interesting part here.
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.
In a real application, these services would be used in other services, in console commands, etc. You cannot remove them.
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 agree than in a real app large app I would explicitly defined at least the slugger
service. However, it's not necessary in this sample RAD app and it shows how the new controller system can be used (for RAD only of course).
Just some hints:
- those services hey are used in other services here (
@markdown
is used in@app.twig.app_extension
) - an advantage of using autowiring is if you change signature of the constructor of the
AppExtension
or if you rename themarkdown
service, you don't need to update your config - if the service using
@markdown
is itself autowired the@markdown
is automatically injected (@app.twig.app_extension
) - defining explicitly the service (adding back the
@slugger
definition for instance) doesn't break anything (the named service will be detected and injected everywhere the autowiring service is defined) - by convention, all services created by the autowiring system are accessible using the
autowired.the\fqn
name (but it's better to name them explicitly if you want to access them). Here there is a@autowired.AppBundle\Utils\Slugger
service.
Btw, console commands can be autowired the same way than controller, I'm adding an example right now.
@@ -5,8 +5,8 @@ | |||
# the routes in YAML, XML or PHP files. | |||
# See http://symfony.com/doc/current/book/routing.html | |||
app: | |||
resource: '@AppBundle/Controller/' | |||
type: annotation | |||
resource: '@AppBundle/Action/' |
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.
IMO, keeping the controllers in Controller
is better, even if you rely on a special loader aware of the way they are registered as services.
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.
Yes I've opened an alternative PR for that. But I think I'll do it even in the default setup of the bundle. It will be more familiar for Symfony developers (and it seems to be a general concern).
I'm closing this as "won't merge" because the initial idea of enabling DunglasActionBundle by default on the Symfony Standard edition was abandoned. Symfony Demo must be "Symfony Standard compliant", so we can't make this change by default in the Symfony Demo. Thanks. |
@javiereguiluz AFAIK this is not rejected yet (symfony/symfony-standard#960). |
Follows symfony/symfony-standard#959