Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[DependencyInjection] Add support for short services configurators syntax#19190
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
| returnarray($this->resolveServices('@'.$parts[0]),$parts[1]); | ||
| } | ||
| return$callable; |
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.
elseif can be a simpleif.
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.
@hhamon, You're right, with standaloneif code is more readable. I've changedelseif toif. Thanks!
ro0NL commentedJun 27, 2016
What about |
nicolas-grekas commentedJun 27, 2016
As far as I understand correctly, this continues#12008 that missed the "configurator" case? |
voronkovich commentedJun 27, 2016
@ro0NL, I would prefer to use |
ro0NL commentedJun 27, 2016 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
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 |
voronkovich commentedJun 27, 2016
@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 |
stof commentedJun 27, 2016
@voronkovich I don't understand what you mean. |
stof commentedJun 27, 2016
I'm 👍 for making the configurator configuration support the same short syntax than the factory one in Yaml. Being consistent is a good idea |
voronkovich commentedJun 27, 2016
@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 |
stof commentedJun 27, 2016
@voronkovich if there is a new supported syntax, we need to have a test ensuring that we don't break this syntax by mistake |
voronkovich commentedJun 27, 2016
@stof, Thanks for the explanation!@nicolas-grekas, I've added the tests and fixed some errors. |
nicolas-grekas commentedJun 28, 2016
👍 |
fabpot commentedJun 29, 2016
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 | MITThis PR adds support for short services configurators syntax in YAML files:```yamlservices: 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
nicolas-grekas commentedMay 20, 2019 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
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#11589This 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#19190Commits-------f8a04fd [DI] deprecate short callables in yaml
This PR adds support for short services configurators syntax in YAML files: