Removed deprecations in DependencyInjection component #14155
Conversation
dosten
commented
Apr 1, 2015
| Q | A |
|---|---|
| Bug fix? | no |
| New feature? | no |
| BC breaks? | no |
| Deprecations? | no |
| Tests pass? | yes |
| Fixed tickets | - |
| License | MIT |
| Doc PR | - |
|
The tests didn't run, @fabpot can you run them manually? |
| @@ -681,72 +672,11 @@ private function addServices() | ||
| } else { | ||
| $privateServices .= $this->addService($id, $definition); | ||
| } | ||
|
|
||
| $synchronizers .= $this->addServiceSynchronizer($id, $definition); |
There was a problem hiding this comment.
The $synchronizers variable can also be removed as its not really used anymore now.
On line 666 ( 😈 ) and 677
|
LGTM 👍 |
| @@ -148,7 +148,7 @@ private function isInlineableDefinition(ContainerBuilder $container, $id, Defini | ||
| return false; | ||
| } | ||
|
|
||
| if (count($ids) > 1 && $definition->getFactoryService(false)) { | ||
| if (count($ids) > 1 && $definition->getFactory()) { |
There was a problem hiding this comment.
this should be removed entirely instead. The case of factories is already covered by the previous condition
|
this is likely to conflict with #14127 |
|
Thank you @dosten. |
…(dosten) This PR was squashed before being merged into the 3.0-dev branch (closes #14155). Discussion ---------- Removed deprecations in DependencyInjection component | Q | A | ------------- | --- | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - Commits ------- 76ff5a9 Removed deprecations in DependencyInjection component
| // $this, so ignore this call. | ||
| // @todo Throw InvalidArgumentException in next major release. | ||
| return; | ||
| throw new InvalidArgumentException('You cannot set service "service_container".'); |
There was a problem hiding this comment.
this and also the special logic in has looks wrong to me. the container should not know anything about the service_container anymore and just treat it like any other service.
There was a problem hiding this comment.
nvm, it's purpose is to not be able to overwrite the special service which is ok
There was a problem hiding this comment.
Hm, do we actually need this kind of special service at all or could we get rid of it?
There was a problem hiding this comment.
@xabbuh I think we need it to inject the entire container into a service (not recommended I know but a lot of developers do)
|
@dosten very nice work! |
This PR was merged into the 3.0-dev branch. Discussion ---------- [DependencyInjection] Remove deprecated code | Q | A | ------------- | --- | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | License | MIT This code has been removed in #14155 but seems that a wrong merge didn't removed it. Commits ------- f2fef91 Removed deprecated code