doctrine / DoctrineMongoDBBundle

Integrates Doctrine MongoDB ODM with Symfony
http://symfony.com/doc/current/bundles/DoctrineMongoDBBundle/index.html
MIT License
377 stars 231 forks source link

Deprecate modifying class parameters #811

Closed franmomu closed 6 months ago

franmomu commented 7 months ago

Ref https://github.com/doctrine/DoctrineMongoDBBundle/pull/810#discussion_r1425609444

@GromNaN do you mean something like this?

GromNaN commented 7 months ago

Just noted that we can deprecate parameters.

You can also deprecate container parameters in your extension to warn users about not using them anymore. This helps with the migration across major versions of an extension. Deprecation is only possible when using PHP to configure the extension, not when using XML or YAML. Use the ContainerBuilder::deprecateParameter() method to provide the deprecation details:

   $containerBuilder->setParameter('acme_demo.database_user', $configs['db_user']);

   $containerBuilder->deprecateParameter(
       'acme_demo.database_user',
       'acme/database-package',
       '1.3',
       // optionally you can set a custom deprecation message
       '"acme_demo.database_user" is deprecated, you should configure database credentials with the "acme_demo.database_dsn" parameter instead.'
   );
franmomu commented 7 months ago

Just noted that we can deprecate parameters.

You can also deprecate container parameters in your extension to warn users about not using them anymore. This helps with the migration across major versions of an extension. Deprecation is only possible when using PHP to configure the extension, not when using XML or YAML. Use the ContainerBuilder::deprecateParameter() method to provide the deprecation details:

   $containerBuilder->setParameter('acme_demo.database_user', $configs['db_user']);

   $containerBuilder->deprecateParameter(
       'acme_demo.database_user',
       'acme/database-package',
       '1.3',
       // optionally you can set a custom deprecation message
       '"acme_demo.database_user" is deprecated, you should configure database credentials with the "acme_demo.database_dsn" parameter instead.'
   );

I didn't know about that, but apparently it's only available since 6.3: https://github.com/symfony/symfony/pull/47719

GromNaN commented 7 months ago

Ok, so we can detect if method_exists($containerBuilder, 'deprecateParameter') and add the deprecation in that case. Developers who want to upgrade to DoctrineMongoDBBundle 5 should update to Symfony 6.4+ before.

That would be more simple than the conditional warning added in this PR.

malarzm commented 7 months ago

ContainerBuilder::deprecateParameter() sounds awesome :) I'd suggest adding an additional note in UPGRADE file and we should be good.

Developers who want to upgrade to DoctrineMongoDBBundle 5 should update to Symfony 6.4+ before.

This is a good point 👍

franmomu commented 7 months ago

After trying this, not sure if we can use ContainerBuilder::deprecateParameter() because we are still using these parameters so apparently we cannot deprecate them with this method without triggering deprecations 😞

malarzm commented 7 months ago

Weird, how Symfony suggests to use this deprecation then?

GromNaN commented 7 months ago

Weird, how Symfony suggests to use this deprecation then?

Maybe we can deprecate the parameters only in the next major version, once they are no longer used.

GromNaN commented 6 months ago

Thank you @franmomu