Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

[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

Merged
fabpot merged 1 commit intosymfony:masterfromnicolas-grekas:di-deprec_
Feb 16, 2017

Conversation

@nicolas-grekas
Copy link
Member

QA
Branch?master
Bug fix?no
New feature?no
BC breaks?no
Deprecations?yes
Tests pass?no
Fixed tickets-
LicenseMIT
Doc PR-

As discussed when introducing_defaults

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);
Copy link
Member

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).

Copy link
MemberAuthor

@nicolas-grekasnicolas-grekasFeb 1, 2017
edited
Loading

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.

wouterj, chalasr, and linaori reacted with thumbs up emoji
@jvasseur
Copy link
Contributor

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
Copy link
MemberAuthor

We wondered about tags for eg_defaults in#21194, but that didn't look good. What would it look like to you?

@jvasseur
Copy link
Contributor

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
Copy link
MemberAuthor

That's what I meant by:

that didn't look good

I keep this opinion :)

@jvasseur
Copy link
Contributor

jvasseur commentedFeb 1, 2017
edited
Loading

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
Copy link
Member

Is it too late to create a new config option calledservices_defaults ? It could convey its meaning better and it'd remove the need for hacks and for the YAML/XML unparity:

services_defaults:public:falseservices:Foo\Bar:[@foo]

@nicolas-grekas
Copy link
MemberAuthor

@javiereguiluz this discussion already happened, see#21071 :)

@stof
Copy link
Member

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
Copy link
Member

Thank you@nicolas-grekas.

@fabpotfabpot merged commit7781082 intosymfony:masterFeb 16, 2017
fabpot added a commit that referenced this pull requestFeb 16, 2017
…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
@nicolas-grekasnicolas-grekas deleted the di-deprec_ branchFebruary 16, 2017 12:54
@fabpotfabpot mentioned this pull requestMay 1, 2017
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@javiereguiluzjaviereguiluzjaviereguiluz left review comments

Assignees

No one assigned

Projects

None yet

Milestone

3.3

Development

Successfully merging this pull request may close these issues.

6 participants

@nicolas-grekas@jvasseur@javiereguiluz@stof@fabpot@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp