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

Appearance settings

[DI] Populate class of ChildDefinition when its id matches an existing FQCN #22362

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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 11, 2017

Conversation

nicolas-grekas
Copy link
Member

@nicolas-grekas nicolas-grekas commented Apr 10, 2017

Q A
Branch? 3.3
Bug fix? no
New feature? no
BC breaks? yes
Deprecations? no
Tests pass? yes
Fixed tickets #22345
License MIT
Doc PR -

See linked issue for expected DX.
Instead of doing a "continue", let's throw to force the resolution of any ambiguities.
There is a minor potential BC Break, if one uses an ambiguous FQCN as child-definition id in 3.2 or below.
To me, this should be rare, and easy to fix (compile time only).
The DX enhancement this PR provides looks worth it.

Service definition "App\Foo\Child" has a parent but no class, and its name looks like a FQCN. Either the class is missing or you want to inherit it from the parent service. To resolve this ambiguity, please rename this service to a non-FQCN (e.g. using dots), or create the missing class.

Copy link
Contributor

@pierredup pierredup left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Copy link
Contributor

@Nek- Nek- left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@fabpot
Copy link
Member

fabpot commented Apr 11, 2017

Thank you @nicolas-grekas.

@fabpot fabpot merged commit 3200d37 into symfony:master Apr 11, 2017
fabpot added a commit that referenced this pull request Apr 11, 2017
…ches an existing FQCN (nicolas-grekas)

This PR was merged into the 3.3-dev branch.

Discussion
----------

[DI] Populate class of ChildDefinition when its id matches an existing FQCN

| Q             | A
| ------------- | ---
| Branch?       | 3.3
| Bug fix?      | no
| New feature?  | no
| BC breaks?    | yes
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #22345
| License       | MIT
| Doc PR        | -

See linked issue for expected DX.
Instead of doing a "continue", let's throw to force the resolution of any ambiguities.
There is a minor potential BC Break, if one uses an ambiguous FQCN as child-definition id in 3.2 or below.
To me, this should be rare, and easy to fix (compile time only).
The DX enhancement this PR provides looks worth it.

> Service definition "App\Foo\Child" has a parent but no class, and its name looks like a FQCN. Either the class is missing or you want to inherit it from the parent service. To resolve this ambiguity, please rename this service to a non-FQCN (e.g. using dots), or create the missing class.

Commits
-------

3200d37 [DI] Populate class of ChildDefinition when its id matches an existing FQCN
@nicolas-grekas nicolas-grekas deleted the di-child-class branch April 11, 2017 18:43
@fabpot fabpot mentioned this pull request May 1, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants
Morty Proxy This is a proxified and sanitized view of the page, visit original site.