Add documentation for WordPress.NamingConventions.ValidVariableName#2457
Conversation
jrfnl
left a comment
jrfnl
left a comment
There was a problem hiding this comment.
Hi @richardkorthuis, thanks for working on this PR and my apologies that it took me a while to get to reviewing it.
On the whole, looking good!
There are just two small remarks/questions:
- The inline remark about the second code comparison blocks missing the
<em>tags. - As the sniff flags both variable _declaration_s as well as the use of variables which don't comply, I'm wondering if it would make sense to add a second code sample to the code comparison with an
echostatement using a variable or a function call being passed a variable ?
Co-authored-by: Juliette <663378+jrfnl@users.noreply.github.com>
Co-authored-by: Juliette <663378+jrfnl@users.noreply.github.com>
Co-authored-by: Juliette <663378+jrfnl@users.noreply.github.com>
Co-authored-by: Juliette <663378+jrfnl@users.noreply.github.com>
Co-authored-by: Juliette <663378+jrfnl@users.noreply.github.com>
Co-authored-by: Juliette <663378+jrfnl@users.noreply.github.com>
…f a declaration).
|
@jrfnl I have added the extra code sample with an echo. |
Thanks for the update @richardkorthuis ! Looks good, I'd just like to combine the third code comparison with the first as they basically highlight the same issue. (A code comparison can contain multiple code samples within each |
|
@richardkorthuis, I was just wondering if you'll have a chance to finish this off in the near future. It would be great if this PR could be included in the next WPCS release. If you haven't got time or lost interest, please let us know and we'll see if we can find someone to take over. Thanks! |
|
@rodrigoprimo Thank you for the reminder, this PR completely lost my focus. I have updated the PR with the combined code comparisons. |
jrfnl
left a comment
jrfnl
left a comment
There was a problem hiding this comment.
@richardkorthuis Thanks for making that update! Looks great. Happy to get this merged.
* upstream/develop: (428 commits) Rulesets: update schema URL GH Actions: use the xmllint-validate action runner and enhance checks (WordPress#2522) AbstractFunctionParameterSniff: fix first class callables and function imports (WordPress#2518) DontExtractStandard.xml file creation (WordPress#2456) Add documentation for WordPress.NamingConventions.ValidVariableName (WordPress#2457) Remove unused variables from a few sniffs (WordPress#2514) I18nTextDomainFixer: remove unnecessary variable initialization (WordPress#2513) GH Actions: Bump codecov/codecov-action from 4 to 5 (WordPress#2510) GH Actions: PHP 8.4 has been released CS/QA: remove redundant condition GH Actions: use explicit PHPStan major Various sniffs: simplify skipping the rest of the file GH Actions: always quote variables Release checklist: add new action item AbstractClassRestrictionsSniff: fix insufficient defensive coding (WordPress#2500) ✨ New WordPress.WP.GetMetaSingle sniff (WordPress#2465) Fix typo in AbstractFunctionRestrictionsSniff::is_targetted_token() DocBlock (WordPress#2477) Fix typos (WordPress#2472) Documentation: capitalization consistency fixes (WordPress#2469) [Documentation]: WordPress.DB.PreparedSQL (WordPress#2454) ...
4 participants
Related to #1722