User tests: Successful: Unsuccessful:
Code review.
This method can and is called many times with a string as the value for $button
E.g:
$bar->appendButton('Custom', $layout->render($options), 'versions');
$bar->appendButton('Standard', 'cancel', $alt, $task, false);
Status | New | ⇒ | Pending |
Category | ⇒ | Libraries |
If the docblocks don't match real life then IDEs like phpStorm and static analysis like psalm will continue to error and moan.
Labels |
Added:
?
|
I have tested this item
I have looked up the toolbar button code quite a bit so approve the docblock clarification that the parameter can be a ToolbarButton or a string, and the deprecation comment.
@brianteeman I don't think we have a proper way for deprecation notes if the deprecation only concerns one of the possible data types of a mixed parameter. But I'm not sure with that. To me it seems ok like it is now in this PR.
It will be ignored by any tooling that reports on code being deprecated
It will be ignored by any tooling that reports on code being deprecated
Yes, I saw. But do you know a way to make it ok for the tools?
It needs a
@deprecated
annotation
Yes, but how is it made for a function parameter with mixed type where only one of the possible types is deprecated? I can't remember having ever seen an example for that.
I don't believe there's any deprecated annotations for parameters. You just write it in the method description (and can choose to use the @note annotation as well) - at least to the best of my knowledge. Not sure where the latest developments on the PSR-5 saga have got to...
I have tested this item
Status | Pending | ⇒ | Ready to Commit |
RTC
@PhilETaylor could you add a note and we are good to merge this, thanks.
Using @note
will not achieve the aim of this PR which is to have the deprecation shown in IDE and static analysis tools. Those tools need the @deprecated
tag.
I have tested this item
This method is not deprecated.
Status | Ready to Commit | ⇒ | Closed |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2021-04-15 15:45:09 |
Closed_By | ⇒ | PhilETaylor | |
Labels |
Added:
?
|
You can but you shouldn't because it's deprecated maybe comment this in the parameter description?