-
Notifications
You must be signed in to change notification settings - Fork 10
Set SimpleBus bundle as default command bus #162
Set SimpleBus bundle as default command bus #162
Conversation
| new Knp\Bundle\PaginatorBundle\KnpPaginatorBundle(), | ||
| new Knp\Bundle\MenuBundle\KnpMenuBundle(), | ||
| new Eo\AirbrakeBundle\EoAirbrakeBundle(), | ||
| new SimpleBus\SymfonyBridge\SimpleBusCommandBusBundle(), |
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.
you will need all 3 to get doctrine working properly
new SimpleBus\SymfonyBridge\DoctrineOrmBridgeBundle(),
new SimpleBus\SymfonyBridge\SimpleBusCommandBusBundle(),
new SimpleBus\SymfonyBridge\SimpleBusEventBusBundle(),
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.
What do the other 2 do? Because I didn't need them when developing my bundle.
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.
it wraps the handling of commands inside a database transaction
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.
Do you need config for that, or is it just automatically when you add it in AppKernel?
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.
it happens automatically, no extra configuration needed just add the bundles
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.
@carakas I only included the minimal requisites to be able to use the command bus. But maybe it is a good idea to initialise the doctrine bundle as well, because it is quite handy to use the rollback functionality.
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.
@ohvitorino this hasn't changed yet
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.
Already added the missing doctrine bridge dependencies.
composer.json
Outdated
| "sumocoders/framework-multi-user-bundle": "^4.0", | ||
| "knplabs/knp-paginator-bundle": "^2.5" | ||
| "knplabs/knp-paginator-bundle": "^2.5", | ||
| "simple-bus/symfony-bridge": "^4.1" |
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.
you also need to add simple-bus/doctrine-orm-bridge
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.
Same, didn't need this, what does it do?
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.
I don't remember in exactly what situations but in some if you don't have this the deploys fail, it is also for that reason included in fork
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.
it will also automatically flush at the end of the command
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.
Ah, didn't know that
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.
@carakas according to the documentation, the added bundle already includes the doctrine-orm-bundle (https://github.com/SimpleBus/SymfonyBridge/blob/master/README.md). So no need to include it as well.
|
Some test fail as well |
|
@jonasdekeukelaere my bad. I wasn't expecting to have broken tests just because of installing a bundle :P I'll have a look at it. |
| new Knp\Bundle\PaginatorBundle\KnpPaginatorBundle(), | ||
| new Knp\Bundle\MenuBundle\KnpMenuBundle(), | ||
| new Eo\AirbrakeBundle\EoAirbrakeBundle(), | ||
| new SimpleBus\SymfonyBridge\SimpleBusCommandBusBundle(), |
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.
@ohvitorino this hasn't changed yet
2a4aaf9 to
0a9e549
Compare
This is a dependency of the DoctrineOrmBundle.
This seams to also be required.
I chose to only add
SimpleBusCommandBusBundleinstead of any of the other bundles included by the installed package.https://github.com/SimpleBus/SymfonyBridge