[DependencyInjection] Enable deprecating parameters #17741
Conversation
service_container.rst
Outdated
| because it will be removed in one of the next version of your package. | ||
| Deprecating a container parameter can be done as follow:: | ||
|
|
||
| $container->deprecateParameter('vendor.foo', 'vendor-name/package-name', '1.3', '"vendor.foo" is deprecated, you should use "vendor.foo_replacement" instead.'); |
There was a problem hiding this comment.
Maybe replacing vendor.foo to vendor.old_name is more explicit ?
I think we can remove note by adding optionally comment (it's done in many place in doc) WDYT ?
| $container->deprecateParameter('vendor.foo', 'vendor-name/package-name', '1.3', '"vendor.foo" is deprecated, you should use "vendor.foo_replacement" instead.'); | |
| $container->deprecateParameter( | |
| 'vendor.old_param', | |
| 'vendor-name/package-name', | |
| '1.3', | |
| // optionally you can change message, by default it will be 'The parameter "vendor.old_param" is deprecated.' | |
| '"vendor.old_param" is deprecated, you should use "vendor.new_param" instead.' | |
| ); |
There was a problem hiding this comment.
About vendor.old_name I'm not sure, as this could be a parameter that will simply disappear without any "new name". 🤔
However, I think that indenting the example is a good idea! I'm doing it.
Finally, about the comment above the optional message, I would say I'm 👍 on it, but it seems a bit long however. But it may be just me, I'll check examples in the doc!
Edit: updated with comment! I just didn't put the default message, it feels a bit too much to me 😄
There was a problem hiding this comment.
Is 'vendor-name/package-name', '1.3', on the same line is a choice ?
There was a problem hiding this comment.
Nope it's not, updated, thanks! 😄
04ad117 to
225f8ef
Compare
|
ping @HeahDude |
configuration.rst
Outdated
| @@ -58,7 +58,7 @@ Configuration Formats | |||
| ~~~~~~~~~~~~~~~~~~~~~ | |||
|
|
|||
| Unlike other frameworks, Symfony doesn't impose a specific format on you to | |||
| configure your applications, but lets you choose between YAML, XML and PHP. | |||
There was a problem hiding this comment.
Can this be fixed in 5.4?
There was a problem hiding this comment.
Of course, here it is! #17745
service_container.rst
Outdated
| Deprecating a container parameter can be done as follow:: | ||
|
|
||
| $container->deprecateParameter( | ||
| 'vendor.foo', |
There was a problem hiding this comment.
Let's use a more real example and not foo if possible
There was a problem hiding this comment.
Is it better now? 🙂
ea69b78 to
3f6a0b6
Compare
3f6a0b6 to
b669384
Compare
There was a problem hiding this comment.
Thanks @alexandre-daubois for taking care of this :).
I think we miss two important points that should be clear when reading this doc:
- only the
ContainerBuilderallows deprecating a parameter, it cannot be done via configuration (yaml, xml, nor container configurator) - since it is designed to be used in Symfony bundles it should mostly be done in a DI extension or a compiler pass (ultimately in the bundle class itself), and requires the parameter to be defined before deprecating it, otherwise a
ParameterNotFoundExceptionwill be thrown.
| because it will be removed in one of the next version of your package. | ||
| Deprecating a container parameter can be done as follow:: | ||
|
|
||
| $container->deprecateParameter( |
There was a problem hiding this comment.
It is not obvious to me at this point that $container is an instance of ContainerBuilder.
|
Thank you @HeahDude for these precisions! Will help a lot on redacting something clearer. I'll take care of the update soon! 🙂 |
|
Follow-up available here: #17750 |
…xandre-daubois) This PR was merged into the 6.3 branch. Discussion ---------- [DependencyInjection] Enable deprecating parameters #17741 has been closed involuntarily I guess but an unrelated merge 😄 But it's OK as this new iteration is totally different from the other one. Pretty makes sense to have a new PR! I took into account `@HeahDude`'s remarks (#17741 (review)). I think it's better know. Please let me know if you see something else to update! Commits ------- 978d0e8 [DependencyInjection] Enable deprecating parameters
Resolves #17725