Remove PairHistoryManager service definition

[mustanggb]

This definition was added in #163, mostly for neatness/understanding purposes.

However it either forces ORM, which should be optional, or fails linting, due to missing argument.

So I suggest we remove it to resolve both issues, unless someone else know how to specify a definition without triggering composer to install/require ORM.

[johanwilfer]

I've reverted #180 until we figure this out.

Maybe you can share @hhamon how to reproduce the issue (or contribute a test)? So we don't break what was reported here again? #181

[hhamon]

@johanwilfer I have my current client's project I'm working with many static analysis tools registered in my CI. One of the CI static analysis job is to run the lint:container Symfony console command that ensures all services definitions are valid.

I daily update my third party PHP dependencies, so when I upgraded the bundle from 6.0.0 to 6.0.1, the lint:container CI job started to fail because of the invalid service definition.

The way to support what you want would probably be by having a global configuration setting and a DI compiler pass or bundle extension class to complete the service definition wiring. Probably something like:

tbbc_money:
    # default implicit configuration
    persistence:
        entity_manager: doctrine.orm.default_entity_manager

    # This approach to only configure the pair history system to use the ODM
    # while the other services would still use the ORM entity manager.
    pair_history:
        persistence:
            entity_manager: doctrine.odm.default_entity_manager

Within the bundle extension class, you can then complete the tbbc_money.pair_history_manager service definition to map to the corresponding ORM/ODM entity manager:

class TbbcMoneyExtension extends Extension
{
    public function load(array $configs, ContainerBuilder $container): void
    {
        // ...

        $ormOrOdmServiceId = $config['pair_history']['persistence']['entity_manager'] ?? $config['persistence']['entity_manager'];

        $definition = $container->getDefinition('tbbc_money.pair_history_manager');
        $definition->setArgument(0, new Reference($ormOrOdmServiceId));
    }
}

WDYT?

[johanwilfer]

Thanks a lot for the pointers @hhamon :)

I don't have time to prepare a PR or test this out for the moment, but will return to it eventually. So for I never had a need for a compiler pass so new area to me. Or maybe if you have time @mustanggb if you really don't want to have the ORM optional, this seems like the way to go to make it so.