-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[DI] Overriding services autowired by name under _defaults bind not working #29944
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
Conversation
Typo in PR headline: - [DI] Fixes: #28326 - Overriding services autowired by name under _deaults bind not working
+ [DI] Fixes: #28326 - Overriding services autowired by name under _defaults bind not working |
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.
This is great 👍 I'm only concerned about the PhpFileLoader
which doesn't call FileDefinition::setDefinition()
(https://github.com/przemyslaw-bogusz/symfony/blob/85a28f1e4e5a6197bbc4ed0550b9ce68936c83bf/src/Symfony/Component/DependencyInjection/Loader/Configurator/ServiceConfigurator.php#L65).
Shouldn't the logic be added there too?
See comment in #29949: there may be an issue that makes the container load all classes at compile time. We should avoid it if confirmed. |
85a28f1
to
3bbf849
Compare
@GuilhemN Thanks for the suggestion. I added the funcionality for @nicolas-grekas This PR is rather simple so I don't think I've made a change that would cause
At least, I'm not aware of that. |
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.
cannot test right now,here is a very light review
…er _defaults bind not working
3bbf849
to
5cf6a3f
Compare
5cf6a3f
to
1ad8e81
Compare
1ad8e81
to
7e805ea
Compare
Thank you @przemyslaw-bogusz. |
… bind not working (przemyslaw-bogusz, renanbr) This PR was merged into the 3.4 branch. Discussion ---------- [DI] Overriding services autowired by name under _defaults bind not working | Q | A | ------------- | --- | Branch? | 3.4 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #28326 | License | MIT This is an implementation of ideas and suggestions of @nicolas-grekas and @GuilhemN. Commits ------- 7e805ea more tests 35a40ac [DI] Fixes: #28326 - Overriding services autowired by name under _defaults bind not working
…(teohhanhui) This PR was merged into the 3.4 branch. Discussion ---------- Fix name and phpdoc of ContainerBuilder::removeBindings | Q | A | ------------- | --- | Branch? | 3.4 | Bug fix? | yes | New feature? | no | BC breaks? | no <!-- see https://symfony.com/bc --> | Deprecations? | no | Tests pass? | yes | Fixed tickets | #29944 (comment) | License | MIT | Doc PR | N/A <!-- Write a short README entry for your feature/bugfix here (replace this comment block.) This will help people understand your PR and can be used as a start of the Doc PR. Additionally: - Bug fixes must be submitted against the lowest branch where they apply (lowest branches are regularly merged to upper ones so they get the fixes too). - Features and deprecations must be submitted against the master branch. --> Commits ------- c93194d Fix name and phpdoc of ContainerBuilder::removeBindings
This is an implementation of ideas and suggestions of @nicolas-grekas and @GuilhemN.