PHP/DevelopmentFunctions: add XML documentation#2690
Conversation
- Change standard descriptions to be more direct. - Remove function lists from standard descriptions. - Fix "(Not)" in titles to "Not using...". - Make titles and comments more generic. - Fix typo `php_info()` to `phpinfo()`. - Use "full path disclosure" consistently.
rodrigoprimo
force-pushed
the
docs-php-development-functions
branch
from
aaa01b2 to
0ea73de
Compare
jrfnl
left a comment
jrfnl
left a comment
There was a problem hiding this comment.
Thanks for taking this on @rodrigoprimo !
When looking at these docs, I have a feeling that the code comparisons don't actually add any value in this case.
There is precedent for XML docs only containing <standard> blocks (for example), so I wonder if we should consider that for this sniff.
Maybe in combination with one or two examples of the discouraged functions in the <standard> description ? (not the complete list to prevent a maintenance nightmare as you rightly pointed out, but just a few examples).
|
Thanks for the review, @jrfnl! I first removed the This PR is ready for another look when you get a chance. |
|
@jrfnl, I posted the comment above before realizing you had already reviewed my first commit and approved the PR. Let me know which approach you prefer. Happy to revert to the previous one. |
|
I have no strong preference between these two versions. Maybe the second reviewer does. |
4 participants
Description
This PR adds XML documentation for the
WordPress.PHP.DevelopmentFunctionssniff.The documentation is based on the work started by @gogdzl in #2490. I used the original commits and then made subsequent changes, mostly based on the review left in #2490.
I suggest squashing those commits before merging. I'm opening the PR without doing that to make it easier to tell my changes apart from the original changes.
I opted to remove the function lists from the standard descriptions to avoid the maintenance burden of keeping docs in sync when functions are added or removed. Also, this approach is consistent with other similar sniffs like
DB/RestrictedFunctionsandWP/DeprecatedFunctionsthat also don't list all flagged functions.Suggested changelog entry
N/A
Related issues/external references
Related to: #1722
Supersedes: #2490
Closes: #2490