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] Fix service autowiring inheritance#19643
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
dunglas commentedAug 16, 2016
👍 |
weaverryan commentedAug 17, 2016
There's technically a minor BC break: a service whose parent is autowired, would not have been autowired before, but now it would be. But, except for anoptional dependency in the constructor, instantiation of that child would have failed, because it would be missing its autowired dependencies. So, the BC break is for child services that have optional, type-hinted constructor args: the container will now try to autowire those, but it didn't before. Still, this is a bug and that's seems like areal edge-case. I think we should fix it. If I'm right, should be update the CHANGELOG or UPGRADE doc? 👍 |
22fd618 to143c5c1Comparechalasr commentedAug 17, 2016
CHANGELOG updated, thanks@weaverryan. |
143c5c1 to04ce15dComparejakzal commentedAug 18, 2016
👍 Although i wouldn't treat it as a BC break. Technically it should either be a bugfix or a BC break :) |
chalasr commentedAug 18, 2016
@jakzal Let me know if I can do something for that. |
| 2.8.9 | ||
| ----- | ||
| *[BC BREAK] fixed inheritance of the`autowire` flag |
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.
Like@jakzal, I think it's more a bug than a a BC break. Anyway, we should either remove the[BC BREAK] here or merge this PR on master.
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.
The BC break flag has been removed as you and@jakzal seem to agree for treat it as a bug fix only.
04ce15d toc49f056Compare| ----- | ||
| * fixed inheritance of the`autowire` flag | ||
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.
The whole paragraph should be removed as the CHANGELOG is automatically generated.
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.
done
Update Changelog
c49f056 tofb95bdcComparefabpot commentedAug 19, 2016
Thank you@chalasr. |
…chalasr)This PR was merged into the 2.8 branch.Discussion----------[DependencyInjection] Fix service autowiring inheritance| Q | A| ------------- | ---| Branch? | 2.8| Bug fix? | yes| New feature? | no| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets |#19615| License | MIT| Doc PR | n/aThis makes services inherit the `autowire` attribute from their parent and fix the ability to override it from the child.Fixed cases:- Simple inheritance```yamlparent: class: Foo abstract: true autowire: truechild: class: Foo```- Set in the child (only)```yamlparent: class: Foo abstract: truechild: class: Foo autowire: true```- Set in the parent, changed in the child```yamlparent: class: Foo abstract: true autowire: truechild: class: Foo autowire: false```Commits-------fb95bdc [DIC] Fix service autowiring inheritance
This makes services inherit the
autowireattribute from their parent and fix the ability to override it from the child.Fixed cases: