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: restore commenting sniffs to PHPCS ruleset #2976

Merged

Conversation

justlevine
Copy link
Collaborator

What does this implement/fix? Explain your changes.

This PR restores the Squiz.Commenting PHPCS ruleset, and addresses the resulting smells.
More specifically:

  • Squiz.Commenting.BlockComment, Squiz.Commenting.DocCommenting.Alignment, Squiz.FunctionComment.SpacingAfterParam, and Squiz.Commenting.InlineComment.NoSpaceBefore were autofixed.
  • added missing and fixed existing@var tags, @params and their types, @throws tags.
  • fixed bad file comment formatting
  • (smaller changes are listed as individual commits).

See additional comments for the remaining Squiz.Commenting smells that are now explicitly excluded.

No production code has been changed in this PR

Does this close any currently open issues?

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

Any other comments?

  • Squiz.Functions.MultiLineFunctionDeclaration.SpaceAfterFunction has been removed from the ruleset as it is already included by WordPress.
  • Generic.Commenting.DocComment.MissingShort has been reclassified as "Should probably be added back".
  • Squiz.Commenting.FunctionComment.IncorrectTypeHint throws false positives when used in conjunction with FCQN. which is significantly more important in terms of IDE hinting.
  • Squiz.Commenting.ClassComment.Missing, Squiz.Commenting.FileComment.Missing. These don't affect IDE hinting, and a lot of effort to add.
  • Squiz.Commenting.FunctionComment.EmptyThrows, Squiz.Commenting.FunctionComment.MissingParamComment. While adding param explanations would be nice, this would be a lot of work to remediate. Saving for a future PR.
  • Squiz.Commenting.InlineComment.InvalidEndChar, would be a lot of work to remediate for only formatting.

Where has this been tested?

Operating System: Ubuntu 20.04 (wsl2 + devilbox + php8.1.15)

WordPress Version: 6.3.2

@justlevine justlevine added Status: In Review Needs to be reviewed by a project maintainer before merge Needs: Reviewer Response This needs the attention of a codeowner or maintainer. Type: Chore (updating CI tasks etc; no production code change) scope: code quality Affects codebase complexity and maintainability labels Nov 1, 2023
@justlevine justlevine changed the title core: restore commenting sniffs to PHPCS ruleset chore: restore commenting sniffs to PHPCS ruleset Nov 1, 2023
@coveralls
Copy link

Coverage Status

coverage: 84.756%. remained the same when pulling 8c8b095 on justlevine:chore/restore-commenting-sniffs into ef67e99 on wp-graphql:develop.

src/Data/PostObjectMutation.php Outdated Show resolved Hide resolved
Co-authored-by: Jason Bahl <jasonbahl@mac.com>
* delete)
* @param \WPGraphQL\AppContext $context The AppContext passed down to all resolvers
* @param int $post_id $post_id The ID of the postObject being
* mutated
Copy link
Collaborator

Choose a reason for hiding this comment

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

this line looks weird too?

Copy link
Collaborator

Choose a reason for hiding this comment

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

🤷🏻‍♂️ maying it's just the Github UI that makes it look off?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Nah these indeed got messed up. 1 sec will push a fix.

Copy link

codeclimate bot commented Nov 1, 2023

Code Climate has analyzed commit f21a3ec and detected 0 issues on this pull request.

View more on Code Climate.

@jasonbahl jasonbahl merged commit 62d8f2b into wp-graphql:develop Nov 1, 2023
25 of 26 checks passed
@justlevine justlevine deleted the chore/restore-commenting-sniffs branch November 1, 2023 21:24
@jasonbahl jasonbahl mentioned this pull request Nov 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs: Reviewer Response This needs the attention of a codeowner or maintainer. 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

3 participants