Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork5.2k
[Contributing] [Standards] Add note abouttrigger_error() and deprecation messages#5840
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Thanks for proposing this change Javier. However, we already have an example inhttp://symfony.com/doc/current/contributing/code/conventions.html#deprecations. Isn't that enough? |
Thank you for the feedback@xabbuh. That example is what I've referenced before. |
I think you should then also add a link to the other section. |
@xabbuh, I don't know how to create a link to another doc's anchor. Moreover, I can't find anything documented atAdding Links section. Could you please give me some clue? |
Sorry for not replying earlier. I completely missed your question. You can link to that section by using the sections label with the Read more at:ref:`contributing-code-conventions-deprecations`. |
Thank you@xabbuh. I've updated the PR with your feedback. |
| @@ -1,2 +1,3 @@ | |||
| /_build | |||
| *.pyc | |||
| /nbproject/private/ | |||
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.
this should be reverted
…cation messages| Q | A| ------------- | ---| Doc fix? | no| New docs? | no| Applies to | 2.3+| Fixed tickets |Add note and example on how to trigger proper deprecation messages.
@xabbuh, comments addressed. |
| * Exception and error message strings should be concatenated using :phpfunction:`sprintf`. | ||
| * Calls to :phpfunction:`trigger_error` with type ``E_USER_DEPRECATED`` should be | ||
| switched to opt-in via ``@`` operator. |
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.
Just FYI, this line break will not be visible.
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.
Sure@xabbuh, it's just my soft limit. Thanks.
👍 |
| */ | ||
| public function someDeprecatedMethod() | ||
| { | ||
| @trigger_error(sprintf('The %s() method is deprecated since version 2.8 and will be removed in 3.0. Use Acme\Baz::someMethod() instead.', __METHOD__), E_USER_DEPRECATED); |
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.
Afaik, sprintf isn't used for deprecations in the code, is it?
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.
It's mixed. Sometimes we use it, sometimes we don't.
@symfony/deciders What do you think? Should we advise to usesprintf()?
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.
👍 to usesprintf, it is coherent with our recommandation for exception messages.
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.
It would be nice to normalize all the calls in some point.
This looks good to me, and it's a nice addition. Thanks Javier! |
…)` and deprecation messages (phansys)This PR was merged into the 2.3 branch.Discussion----------[Contributing] [Standards] Add note about `trigger_error()` and deprecation messages| Q | A| ------------- | ---| Doc fix? | no| New docs? | no| Applies to | 2.3+| Fixed tickets |Add note and example on how to trigger proper deprecation messages.Related to#5525.Commits-------4c2f1fb [Contributing] [Standards] Add note about `trigger_error()` and deprecation messages
Thank you too Ryan! |
Add note and example on how to trigger proper deprecation messages.
Related to#5525.