Skip to content
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

WPCS 3.0: Fix coding standards of VIPCS sniffs #733

Merged
merged 2 commits into from
Jan 25, 2023
Merged

WPCS 3.0: Fix coding standards of VIPCS sniffs #733

merged 2 commits into from
Jan 25, 2023

Conversation

GaryJones
Copy link
Contributor

@GaryJones GaryJones commented Dec 20, 2022

Once #732 has been merged, this can be rebased to just show coding standards changes.


Update the VIPCS sniffs to fix the coding standards changes with WPCS 3.0.

CS: Use pre-increment over post-increment

Slightly better performance.

CS: Avoid reserved keyword for function param name

Using named parameters with a variable named $string is unfavourable.

CS: Update WordPress-Extra exclusions

WordPress-Extra will no longer use the Generic.Arrays.DisallowShortArraySyntax rule, but does use the improved Universal.Arrays.DisallowShortArraySyntax from PHPCSExtra instead, so whilst we have short array syntax being used in the VIPCS sniffs, let's continue to exclude the rule.

@GaryJones GaryJones added this to the 3.x milestone Dec 20, 2022
@GaryJones GaryJones self-assigned this Dec 20, 2022
@GaryJones GaryJones requested a review from a team as a code owner December 20, 2022 02:17
Copy link
Collaborator

@jrfnl jrfnl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@GaryJones Reviewed just the commits which belong to this PR.

IMO...:

  • Commit 1 and 2 (increment + reserved keywords) can/should be rebased on develop and pulled separately. These commits can be merged independently of PR WPCS 3.0: Composer updates #732 as they just apply some best practices (and anticipate that those will be checked for in WPCS 3.x).
  • By having those in a separate PR and merging it ahead of PR WPCS 3.0: Composer updates #732, PR WPCS 3.0: Composer updates #732 has a higher chance of getting a passing build.
  • Commit 3 (ruleset change) should be combined with the commit changing the minimum WPCS version to 3.x in Composer as as soon as 3.x becomes the minimum, the ruleset becomes invalid without the change from this commit.

@GaryJones
Copy link
Contributor Author

Updated to just the two suggested CS-related commits. No longer requires #732.

@GaryJones GaryJones merged commit 5e805d6 into develop Jan 25, 2023
@GaryJones GaryJones deleted the fix/cs branch January 25, 2023 08:11
@jrfnl jrfnl modified the milestones: 3.x, 2.3.4 Mar 5, 2023
@jrfnl
Copy link
Collaborator

jrfnl commented Mar 5, 2023

As this has been merged into develop and develop is not (yet) 3.x, I've moved this to the 2.3.4 milestone.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants