[DependencyInjection] Add support for short services configurators syntax #19190
Conversation
| } | ||
|
|
||
| return $callable; | ||
| } elseif (is_array($callable)) { |
There was a problem hiding this comment.
@hhamon, You're right, with standalone if code is more readable. I've changed elseif to if. Thanks!
|
What about |
|
As far as I understand correctly, this continues #12008 that missed the "configurator" case? |
|
@ro0NL, I would prefer to use |
|
I see. Maybe it's good to mention this is a generic format, ie i thought it was configurator specific at first, but it's also a notation we can use on factories. The BC is due the fact service id's allow for colons? Im not sure how intuitive this (new) format will be..., what about |
|
@ro0NL, it is not a new format. It will be a BC break because some people already use short syntax for service factories in their code without using leading |
|
@voronkovich I don't understand what you mean. |
|
I'm 👍 for making the configurator configuration support the same short syntax than the factory one in Yaml. Being consistent is a good idea |
|
@nicolas-grekas, do we really need additional tests for this? We already have the tests for services factories: https://github.com/symfony/symfony/blob/master/src/Symfony/Component/DependencyInjection/Tests/Loader/YamlFileLoaderTest.php#L157 |
|
@voronkovich if there is a new supported syntax, we need to have a test ensuring that we don't break this syntax by mistake |
|
@stof, Thanks for the explanation! @nicolas-grekas, I've added the tests and fixed some errors. |
|
👍 |
|
Thank you @voronkovich. |
…onfigurators syntax (voronkovich) This PR was squashed before being merged into the 3.2-dev branch (closes #19190). Discussion ---------- [DependencyInjection] Add support for short services configurators syntax | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | License | MIT This PR adds support for short services configurators syntax in YAML files: ```yaml services: app.some_service: class: ... # Common syntax configurator: [ '@app.configurator', 'configure' ] # Short syntax configurator: 'app.configurator:configure' Commits ------- da2757f [DependencyInjection] Add support for short services configurators syntax
|
I propose to deprecate this in #31543 |
This PR was merged into the 4.4-dev branch. Discussion ---------- [DI] deprecate short callables in yaml | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | yes | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | symfony/symfony-docs#11589 This deprecates defining factories as `foo:method` instead of `['@foo', 'method']` in yaml. This is confusing syntax and misses the opportunity to raise an error when one does a typo and uses a single colon instead of two. This basically reverts #19190 Commits ------- f8a04fd [DI] deprecate short callables in yaml
This PR adds support for short services configurators syntax in YAML files: