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

chore: upgrade WPCS to v3.0 #2924

Merged
merged 9 commits into from Sep 13, 2023
Merged

Conversation

justlevine
Copy link
Collaborator

What does this implement/fix? Explain your changes.

This PR updates WPCS and VIPCS to their latest v3.0 versions.

The individual remediation steps are in the commit history.

Does this close any currently open issues?

Any relevant logs, error output, GraphiQL screenshots, etc?

Any other comments?

See the diff comments.

Where has this been tested?

Operating System: Ubuntu 22.04 (wsl2 + devilbox + php 8.1.14)

WordPress Version: 6.3.1

phpcs.xml.dist Outdated Show resolved Hide resolved
phpcs.xml.dist Show resolved Hide resolved
phpcs.xml.dist Outdated Show resolved Hide resolved

<!-- This would be a breaking change to fix-->
<exclude name="WordPress.NamingConventions.ValidFunctionName.MethodNameInvalid" />

<!-- Should probably not be added back -->
<exclude name="PHPCompatibility.Keywords.ForbiddenNamesAsDeclared.objectFound"/>
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This was replaced with Universal.NamingConventions.NoReservedKeywordParameterNames.objectFound and remediated in 66cc23c

@codeclimate
Copy link

codeclimate bot commented Sep 7, 2023

Code Climate has analyzed commit ec93a97 and detected 5 issues on this pull request.

Here's the issue category breakdown:

Category Count
Complexity 1
Duplication 4

View more on Code Climate.

@justlevine
Copy link
Collaborator Author

Commit ec93a97 escapes Exception messages.

It's likely that we're "over-escaping" (many but not all UserErrors are caught and sent via json), but:

  • better safe than sorry
  • the performance impact is negligible (since they usually end the request)
  • It'l make it a lot easier to audit in the future when the sniffs get better or we're more intentional with our usage of Throwables.

Related:
WordPress/WordPress-Coding-Standards#2374

@justlevine justlevine marked this pull request as ready for review September 7, 2023 04:36
@justlevine justlevine added Status: In Review Needs to be reviewed by a project maintainer before merge Type: Chore (updating CI tasks etc; no production code change) scope: code quality Affects codebase complexity and maintainability labels Sep 7, 2023
@jasonbahl jasonbahl merged commit a2d5f4e into wp-graphql:develop Sep 13, 2023
26 of 27 checks passed
@justlevine justlevine deleted the chore/phpcs3 branch September 15, 2023 22:24
@jasonbahl jasonbahl mentioned this pull request Oct 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
scope: code quality Affects codebase complexity and maintainability Status: In Review Needs to be reviewed by a project maintainer before merge Type: Chore (updating CI tasks etc; no production code change)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants