[Contributing] [Standards] Add note about trigger_error() and deprecation messages
#5840
Conversation
|
Thanks for proposing this change Javier. However, we already have an example in http://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 at Adding 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`. |
phansys
force-pushed
the
cs_deprecations
branch
from
03eae82
to
f23b8d3
Compare
|
Thank you @xabbuh. I've updated the PR with your feedback. |
| @@ -1,2 +1,3 @@ | |||
| /_build | |||
| *.pyc | |||
| /nbproject/private/ | |||
…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.
phansys
force-pushed
the
cs_deprecations
branch
from
f23b8d3
to
4c2f1fb
Compare
|
@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.
Just FYI, this line break will not be visible.
There was a problem hiding this comment.
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.
Afaik, sprintf isn't used for deprecations in the code, is it?
There was a problem hiding this comment.
It's mixed. Sometimes we use it, sometimes we don't.
@symfony/deciders What do you think? Should we advise to use sprintf()?
There was a problem hiding this comment.
👍 to use sprintf, it is coherent with our recommandation for exception messages.
There was a problem hiding this comment.
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.