Deprecate constructing AdminPoolLoader with more than one argument #6978
Conversation
franmomu
force-pushed
the
remove_container_from_admin_pool
branch
from
a4106e0
to
774e64f
Compare
franmomu
force-pushed
the
remove_container_from_admin_pool
branch
2 times, most recently
from
4e8bae3
to
8eea762
Compare
|
Digging a bit more about this, in symfony/symfony#36143 the container was added in the router cache, so there is no need for as to add the resource. I've also deprecated passing the |
| @@ -238,9 +238,6 @@ static function (array $a, array $b): int { | |||
| $pool->replaceArgument(1, $admins); | |||
| $pool->replaceArgument(2, $groups); | |||
| $pool->replaceArgument(3, $classes); | |||
|
|
|||
| $routeLoader = $container->getDefinition('sonata.admin.route_loader'); | |||
| $routeLoader->replaceArgument(1, $admins); | |||
There was a problem hiding this comment.
These $admins are the same that the ones injected to the Pool (5 lines above).
franmomu
force-pushed
the
remove_container_from_admin_pool
branch
from
8eea762
to
99de67f
Compare
| ]) | ||
|
|
||
| ->alias(AdminPoolLoader::class, 'sonata.admin.route_loader') | ||
| ->deprecate(...BCDeprecationParameters::forConfig( |
There was a problem hiding this comment.
I've also deprecated the alias since I don't think it's useful, the original service is meant to be loaded as routing.loader, not to inject it in another service.
|
Sorry @phansys 😞 I forgot to add the alias deprecation in the changelog (I think now it's fine). |
franmomu
changed the title
|
@franmomu a rebase is needed :) |
The container was added to the router cache in symfony/framework-bundle in 4.4.6. See symfony/symfony#36143
franmomu
force-pushed
the
remove_container_from_admin_pool
branch
from
99de67f
to
4c42e99
Compare
|
I'll do the merge |
Wait, I was creating the release first |
|
ok, I'll create the PRs to update persistence bundles. |
Subject
The container was added in eefe79c to solve #128, I've marked this PR as draft, as I'm not sure if it is not needed anymore, I'll try to search about this or if someone has some knowledge about this, would be appreciated.
Ref: #6963
I am targeting this branch, because these changes are BC.
Changelog