-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
Leverage trigger_deprecation() from symfony/deprecation-contracts #35550
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
86ace2a
to
df06247
Compare
Given the amount of occurrences of this -> // Before
deprecated($package, $version, $message)
// After
deprecated(string $message, string $package = '', string $version = '') |
actually no: we should improve the deprecation subsystem to allow specifying the package and version of it that introduced eg deprecating this service or this config option. |
src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php
Outdated
Show resolved
Hide resolved
… convention to trigger deprecation notices (nicolas-grekas) This PR was merged into the 5.1-dev branch. Discussion ---------- [Contracts/Deprecation] Provide a generic function and convention to trigger deprecation notices | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | Deprecations? | no | Tickets | - | License | MIT | Doc PR | - This PR results from a discussion we had yesterday with @alcaeus, @beberlei and others at the Symfony UG meetup in Berlin (thanks SensioLabs for bringing me there). It provides a generic function and convention to trigger deprecation notices, which could be a better alternative to the `@trigger_error(..., E_USER_DEPRECATED)` calls that we use currently. This proposed package would provide a single global function named `deprecated()`. Its purpose is to trigger deprecations in a way that can be silenced on production environment by using the `zend.assertions` ini setting and that can be caught during development to generate reports. The function requires at least 3 arguments: - the name of the package that is triggering the deprecation - the version of the package that introduced the deprecation - the message of the deprecation - more arguments can be provided: they will be inserted in the message using `printf()` formatting Example: ```php deprecated('symfony/blockchain', 8.9, 'Using "%s" is deprecated, use "%s" instead.', 'bitcoin', 'fabcoin'); ``` This will generate the following message: `Since symfony/blockchain 8.9: Using "bitcoin" is deprecated, use "fabcoin" instead.` Check #35550 to see how using this function could look like on Symfony itself. Commits ------- f0f41cb [Contracts/Deprecation] Provide a generic function and convention to trigger deprecation notices
df06247
to
5068eb4
Compare
Do we have a some plan for e.g. vendors to provide the package/version details? Currently the ecosystem includes those in the message i tend to believe, as the message is the only way currently. Im talking about deprecated services, config nodes, option resolvers & co. Thus where the user provides deprecation details ... The amount of |
We do, in https://github.com/orgs/symfony/projects/1#card-32681032, help wanted :) |
src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php
Outdated
Show resolved
Hide resolved
5068eb4
to
60b17dc
Compare
src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we sure this should target Symfony 5.1 with new deprecation mechanism? This will break deprecation setup for everybody who doesn't have assertions enabled (pretty much everybody).
Lot of people will probably not notice they stopped getting deprecations, then they will notice in Symfony 5.4, but then it's too late, they meanwhile already mistakenly added lot of deprecations they were not alerted by phpunit-bridge.
In case they do notice it in Symfony 5.1 and do enable assertions, unrelated assertion in vendor package might trigger and they will get errors/warnings that most people will not know how to get rid of without going back to disabled assertions again, leaving their deprecation setup broken.
@ostrolucky the answer is definitely yes, this should be done on 5.1. If there should be any discussion about the exact behavior of the function itself, let's have one, but the goal has to be to merge this PR in 5.1. |
471657f
to
d6bdbae
Compare
d6bdbae
to
18f3622
Compare
18f3622
to
3566e85
Compare
3566e85
to
3e35d8e
Compare
Thank you @nicolas-grekas. |
…n-contracts (nicolas-grekas) This PR was merged into the 5.1-dev branch. Discussion ---------- Leverage trigger_deprecation() from symfony/deprecation-contracts | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | no | Deprecations? | no | Tickets | - | License | MIT | Doc PR | - Here is what it could mean to use `deprecated()` from #35526 in the code base. Commits ------- 3e35d8e Leverage trigger_deprecation() from symfony/deprecation-contracts
Here is what it could mean to use
deprecated()
from #35526 in the code base.