Hopefully improve comments in services.yaml #285
Conversation
nicolas-grekas
commented
Nov 29, 2017
nicolas-grekas
commented
| Q | A |
|---|---|
| License | MIT |
ghost
deployed
to
| @@ -5,24 +5,23 @@ parameters: | |||
| services: | |||
| # default configuration for services in *this* file | |||
| _defaults: | |||
| # automatically injects dependencies in your services | |||
There was a problem hiding this comment.
The current "before the line" comments make the section scary to me
I propose to put these comments at the end of each lines - looks better to the eye IMHO
|
|
||
| # makes classes in src/ available to be used as services | ||
| # this creates a service per class whose id is the fully-qualified class name | ||
| App\: | ||
| resource: '../src/*' | ||
| # you can exclude directories or files | ||
| # but if a service is unused, it's removed anyway |
There was a problem hiding this comment.
not needed anymore, esp since service removal is mentioned above now
| # controllers are imported separately to make sure they | ||
| # have the tag that allows actions to type-hint services | ||
| # controllers are imported separately to make sure services can be injected | ||
| # as action arguments even if you don't extend any base controller class |
There was a problem hiding this comment.
let's explain why this is needed in the first place (ie: only if you do not extend a base controller)
| App\Controller\: | ||
| resource: '../src/Controller' | ||
| tags: ['controller.service_arguments'] | ||
|
|
||
| # add more service definitions when explicit configuration is needed | ||
| # please note that last definitions always *replace* previous ones |
There was a problem hiding this comment.
I think this last note is the most important one, as we've already got several reports from ppl who've got that wrong
There was a problem hiding this comment.
Sadly I don't think this --> "please note that last definitions always replace previous ones" will be enough to understand the problem. The worst thing is that is a tricky problem and not easy to explain: if you define a controller services bellow the App\Controller\ config, it's natural to think that your service will inherit everything from above.
There was a problem hiding this comment.
that's why we absolutely need this line: to hint the user when something doesn't work as one would maybe expect
There was a problem hiding this comment.
Should we include a commented-out example config below?
# Example: override a service so you can explicitly wire a `$isDebug` constructor arg
# App\Service\ExampleService
# arguments:
# $isDebug: '%kernel.debug%'There was a problem hiding this comment.
not sure: we did not add examples for parameters, and I think it was a conscious decision
|
|
||
| # makes classes in src/ available to be used as services | ||
| # this creates a service per class whose id is the fully-qualified class name | ||
| App\: | ||
| resource: '../src/*' | ||
| # you can exclude directories or files | ||
| # but if a service is unused, it's removed anyway | ||
| exclude: '../src/{DataFixtures,Entity,Migrations,Tests}' |
There was a problem hiding this comment.
Remove DataFixtures, we just fixed this with DoctrineFixturesBundle 3.0. Woo!
