-
-
Notifications
You must be signed in to change notification settings - Fork 522
DontExtractStandard.xml file creation #2456
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
fix incomplete description of invalid use case description
jrfnl
left a comment
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.
@aiolachiara Thank you for working on this!
I agree with the remark @GaryJones left previously and have added a few more nitpick comments inline.
I look forward to seeing the next iteration of the PR!
| my_extract( $var_array, EXTR_OVERWRITE ); | ||
| ]]> | ||
| </code> | ||
| <code title="Invalid: use `extract()` function"> |
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.
| <code title="Invalid: use `extract()` function"> | |
| <code title="Invalid: Using the `extract()` function."> |
|
@aiolachiara, 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, I'm sorry for forgetting to complete this PR. I made some fixes to the documentation, and I hope to have included all the suggestions received. |
jrfnl
left a comment
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.
@aiolachiara Thank you for making those updates! All looking good.
Only thing left is to use proper punctuation for the Valid/Invalid title (i.e. add a . at the end of each sentence + add the missing "the" in the Invalid title), but we can fix that up on merge if needs be. Let me know if you want to fix this up yourself still or if you prefer we fix it up on merge.
I fixed punctuation and misspellings, I hope It is fine now. |
jrfnl
left a comment
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.
@aiolachiara Awesome! Thank you for the quick turn-around. All good from me now ✔️
dingo-d
left a comment
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 one comment left, otherwise looks good 👍🏼
* 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) ...
PR WordPress#2456 was missing a milestone and because of that it was not included in the 3.2.0 changelog. This commit fixes that.
Related to #1722