symfony icon indicating copy to clipboard operation
symfony copied to clipboard

Published 20 hours ago • verified publisher icon symfony

[FrameworkBundle][Notifier] Allow to configure or disable the message bus to use

Open jschaedl opened this issue 4 years ago • 17 comments

Q A
Branch? 5.4
Bug fix? no
New feature? yes
Deprecations? no
Tickets -
License MIT
Doc PR tbd

For the mailer integration there is already a possibility to disable or configure the message_bus to use. See: https://github.com/symfony/symfony/issues/34633 and https://github.com/symfony/symfony/pull/34648

This PR introduces a similar config option notifier.message_bus for the notifier integration.

Things done:

  • [x] Add a config option notifier.message_bus for the notifier configuration
  • [x] Adjust FrameworkExtension considering the new message_bus config option
  • [x] Add tests for the new config option

jschaedl avatar Dec 06 '20 15:12 jschaedl

It looks like your commit is not associated with your GitHub account: image

And FrameworkBundle is twice in the PR header 🧐

OskarStark avatar Dec 10 '20 17:12 OskarStark

Anything left todo here @jschaedl ?

OskarStark avatar Jan 30 '21 16:01 OskarStark

Hi @OskarStark Implementation is done. But I have no idea why tests are failing on PHP 7.4 🤔

jschaedl avatar Jan 31 '21 08:01 jschaedl

Can you please rebase and see if it's still failing ? Thanks

OskarStark avatar Jan 31 '21 10:01 OskarStark

@jschaedl can you please check the failing tests?

Thanks

OskarStark avatar Apr 06 '21 11:04 OskarStark

@OskarStark I rebased the branch. Tests are still failing on PHP 8.0 but this seems to be unrelated with my changes.

jschaedl avatar Apr 06 '21 13:04 jschaedl

The failures are related I believe.

nicolas-grekas avatar Apr 13 '21 08:04 nicolas-grekas

Needs rebase after https://github.com/symfony/symfony/pull/40842 was merged

jschaedl avatar Apr 17 '21 07:04 jschaedl

@OskarStark I am about to split this PR.

  1. Adding the missing TransportFactories to the FrameworkBundle: https://github.com/symfony/symfony/pull/40843 ✅
  2. I would like to add basic tests for the Framework integration for the Notifier: https://github.com/symfony/symfony/pull/40844 ✅
  3. Adding a test to make sure we won't forget to add TransportFactories anymore: https://github.com/symfony/symfony/pull/40845 ✅
  4. And then adding the possibility to add a specific message bus with this PR ✅

jschaedl avatar Apr 17 '21 15:04 jschaedl

Rebased and ready for another review :-)

jschaedl avatar May 03 '21 07:05 jschaedl

Ready to merge @nicolas-grekas ?

OskarStark avatar Jun 17 '21 09:06 OskarStark

Please rebase to get rid of Travis, thanks

OskarStark avatar Aug 04 '21 21:08 OskarStark

@OskarStark Rebase done.

jschaedl avatar Aug 17 '21 06:08 jschaedl

Some tests are failing

chalasr avatar Aug 19 '21 21:08 chalasr

@jschaedl Can you have a look at the failing tests?

fabpot avatar Oct 19 '21 07:10 fabpot

@jschaedl I've rebased on current 6.2 and fixed the tests. Can you review my commit before I merge?

fabpot avatar Jul 21 '22 10:07 fabpot

Friendly ping @jschaedl

nicolas-grekas avatar Oct 19 '22 12:10 nicolas-grekas

I rebased again. Looks good to me now.

jschaedl avatar Nov 09 '22 19:11 jschaedl

Thank you @jschaedl.

fabpot avatar Dec 11 '22 08:12 fabpot