-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[FrameworkBundle] message_bus
alias public
#28216
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
ControllerTrait deals with a service locator, services provided by a service locator do not need to be public (and should not). |
hmm, ControllerTrait is used by the FrameworkBundle's |
It's time to deprecate |
In the meantime... this is still a bug to fix 🙃 |
@@ -1488,7 +1488,7 @@ private function registerMessengerConfiguration(array $config, ContainerBuilder | |||
$container->register($busId, MessageBus::class)->addArgument(array())->addTag('messenger.bus'); | |||
|
|||
if ($busId === $config['default_bus']) { | |||
$container->setAlias('message_bus', $busId); | |||
$container->setAlias('message_bus', $busId)->setPublic(true); |
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.
or $container->setAlias('message_bus', new Alias($busId, true));
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.
Yep. using setPublic is more explicit to me :)
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.
Sadly :)
What would be the consequence of removing |
ControllerTrait is internal. The consequence would be to ask people to use AbstractController instead. |
Thank you @sroze. |
This PR was merged into the 4.1 branch. Discussion ---------- [FrameworkBundle] `message_bus` alias public | Q | A | ------------- | --- | Branch? | 4.1 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #28215 | License | MIT | Doc PR | ø Because it is used in the `ControllerTrait` with `get('message_bus')`... same than for `security.csrf.token_manager` and cie, it should be public. Commits ------- 51b6e9e Make the `message_bus` alias public
Oh okay I better use |
Yep. I'll send a PR to deprecate it :)
…On Tue, Aug 21, 2018 at 11:14 AM Mathieu ***@***.***> wrote:
Oh okay I better use AbstractController right now then?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#28216 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAxHEXgVazULdjG3QsEcAyV1ntUeGhEiks5uS8-NgaJpZM4WBdQE>
.
|
Because it is used in the
ControllerTrait
withget('message_bus')
... same than forsecurity.csrf.token_manager
and cie, it should be public.