Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[DI] Deprecate underscore-services in YamlFileLoader#21484
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
| privatefunctionparseDefinition($id,$service,$file,array$defaults) | ||
| { | ||
| if (preg_match('/^_[a-zA-Z0-9_]*$/',$id)) { | ||
| @trigger_error(sprintf('Service names that start with an underscore are deprecated since Symfony 3.3 and will be reserved in 4.0. Rename the "%s" service or define it in XML instead.',$id),E_USER_DEPRECATED); |
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 don't think it's a good idea that the allowed service ids depend on the config format. In the past, we've worked hard to make all formats work the same in spite of their differences (for example, by adding PHP constants support for Yaml files). If we want to deprecate something, we could deprecate just the_defaults string as service id (whatever the config format used).
nicolas-grekasFeb 1, 2017 • 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.
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.
There is little comparison between supporting a feature in all formats (eg constants), and excluding a very specific narrow case for one specific loader which misses a few oops of expressiveness (eg underscore-services, which are not a "feature").
We also stated previously that we don't want parity with all loader formats - quite the contrary in fact: we want to embrace the full expressiveness of each format. Here, it's exactly a matter of expressiveness. Thus falls into this policy to me - thus only the yaml loader should be affected.
jvasseur commentedFeb 1, 2017
Maybe we could use a yaml tag for default values (or other non-service keys) instead and keep allowing anything for service names. |
nicolas-grekas commentedFeb 1, 2017
We wondered about tags for eg |
jvasseur commentedFeb 1, 2017
Probably something like this services:defaults:!defaultpublic:falseFoo\Bar:[@foo] Any service tagged with default would be used as default instead of a service. And the key would be ignored. |
nicolas-grekas commentedFeb 1, 2017
That's what I meant by:
I keep this opinion :) |
jvasseur commentedFeb 1, 2017 • 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.
Yeah I don't like it that much, but using underscore-names to convey special meanings looks to much like a hack to me to like it. |
javiereguiluz commentedFeb 1, 2017
Is it too late to create a new config option called services_defaults:public:falseservices:Foo\Bar:[@foo] |
nicolas-grekas commentedFeb 1, 2017
@javiereguiluz this discussion already happened, see#21071 :) |
stof commentedFeb 1, 2017
@javiereguiluz this isalso a BC break, as it makes this key forbidden for DI extensions (and it is much harder to write a BC layer, as we would be unable to detect this case as the DI extension could expose a config similar to our defaults) |
fabpot commentedFeb 16, 2017
Thank you@nicolas-grekas. |
…nicolas-grekas)This PR was merged into the 3.3-dev branch.Discussion----------[DI] Deprecate underscore-services in YamlFileLoader| Q | A| ------------- | ---| Branch? | master| Bug fix? | no| New feature? | no| BC breaks? | no| Deprecations? | yes| Tests pass? | no| Fixed tickets | -| License | MIT| Doc PR | -As discussed when introducing `_defaults`Commits-------7781082 [DI] Deprecate underscore-services in YamlFileLoader
As discussed when introducing
_defaults