Renamed event_manager to match doctrine-orm-module configuration

DASPRiD / container-interop-doctrine

Doctrine factories for container-interop

107 stars 25 forks source link

Renamed event_manager to match doctrine-orm-module configuration #12

Closed masarliev closed 7 years ago

coveralls commented 8 years ago

Coverage remained the same at 26.992% when pulling 02f17f6a13e9c4ed17899ff452bfdcf3d30c5d63 on masarliev:master into 05041506e35cbc4ddd89374ca5998c3544b14a52 on DASPRiD:master.

DASPRiD commented 8 years ago

This is a BC break which we cannot merge before the next major release.

asgrim commented 7 years ago

@DASPRiD version 1.0.0 hasn't been released yet, I don't see a problem with merging this before 1.0.0?

stefanotorresi commented 7 years ago

👎

it doesn't add any value at all
it introduces inconsistency with the entity_manager service name, and every other config key for that matter, which are all in snake_case. eventmanager would be the only one without the separator.
there is no actual need to bring a convention from another package that is not actually a dependency, but just loosely inspired to. I also think that this was in fact a mistake on doctrine-orm-module part, and it has been kept for BC, so you would basically import a wrong convention.

asgrim commented 7 years ago

@stefanotorresi interesting points made! I guess this is for @DASPRiD to decide as it's his lib...

DASPRiD commented 7 years ago

@stefanotorresi had valid points, and I totally agree with them. Let's stick with what we have.