-
-
Notifications
You must be signed in to change notification settings - Fork 94
Conversation
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 a BC break? You can't do it on 3.x
cc @sonata-project/contributors |
I would rather add deprecation now, change it on master and for now add it to recipe |
|
I changed base branch of this PR |
Please add upgrade note. Should we deprecate this on stable @sonata-project/contributors ? |
What should probably be deprecated on stable would be not specifying this boolean (and expecting it to default to |
May be if class And i do not know how to deprecate configuration ) |
Since #326, even if admin is enabled, services for SonataAdmin are added only if notification backend is |
Thanks @covex-nn |
I am targeting this branch, because this is BC break
Changelog
Subject
I believe, that disabling admin in configuration is more correct, because it can be enabled only when
sonata.notification.backend.doctrine
was enabled.A default configuration, i.e. recipe for
sonata-project/notification-bundle
could be like this:Also, default configuration could contain commented configuration for
backend: sonata.notification.backend.rabbitmq
. Anyway, i have already created PR with recipe for this bundle in febuary symfony/recipes-contrib#302Additional configuration, i.e. a recipe for a new package
sonata-project/notification-orm-pack
could be like this (i created a gist with entity class):So, additional configuration will override default.