search

KnpLabs / KnpRadBundle

Rapid Application Development for Symfony2 [UNMAINTAINED]
MIT License
291 stars 49 forks source link

[Domain Event] Use symfony dispatcher instead of doctrine one #195

Closed gquemener closed 10 years ago

gquemener commented 10 years ago

Using doctrine event manager to dispatch domain events prevents domain event listeners to depend on doctrine (circular reference). Plus, it doesn't make much more sense to dispatch domain events through a storage layer dispatcher.

This commit allows to dispatch domain events through an event dispatcher (it uses the symfony event dispatcher by default).

gquemener commented 10 years ago

What do you think @docteurklein ? I agree that the kernel event dispatcher might not be the best dispatcher to use, but it's definitively better than the doctrine one.

Domain Event ftw! :horse:

docteurklein commented 10 years ago

Seems good to me! Just one thing, the feature suite fails on https://travis-ci.org/KnpLabs/KnpRadBundle/jobs/35732924#L1009 now.

docteurklein commented 10 years ago

Otherwise, here is another impl that uses no third party dispatcher :) even simpler!

https://github.com/docteurklein/event-store/blob/master/src/Knp/Event/Dispatcher.php

gquemener commented 10 years ago

Indeed, it's even simpler but not as integrated within symfony as is the symfony event_dispatcher.

I'm taking a look at the build

docteurklein commented 10 years ago

totally right. we will benefit from its DIC integration (tags, debug command, ... ) +1

gquemener commented 10 years ago

Scenarios should be fixed now.

BTW, this is a BC Break and should be tagged in a major version

docteurklein commented 10 years ago

awwwwesome!

docteurklein commented 10 years ago

it's a BC break because of the DIC tags, right?

docteurklein commented 10 years ago

Could it be documented in an UPGRADE file, and just make a minor release? It's not like we broke, rewrote and changed behavior. it's just its integration with DIC.

gquemener commented 10 years ago

yep because of the tags. Also because Provider events now contains EventDispatcher component Event instances instead of doctrine events.

After all, if you're ok with corrent documentation, let me write it quickly!

gquemener commented 10 years ago

Here it is @docteurklein :beer:

docteurklein commented 10 years ago

awesome! but I'd prefer a single UPGRADE.md file if possible :) I find it confusing to follow a multi-version upgrade if splitted.

docteurklein commented 10 years ago

you rock :)

gquemener commented 10 years ago