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

fix: restore whitespace rules for PHPCBF #291

Merged
merged 4 commits into from
Aug 29, 2024

Conversation

missionmike
Copy link
Contributor

This PR restores some rules that auto-format trailing whitespace and newlines in PHPCBF. I copied the code from: AxeWP/WPGraphQL-Coding-Standards@23dbbaf (thanks @justlevine)

Before

Example from before these rules are reintroduced; the formatter ignores these spacing issues:

image

After

With the rules reintroduced, the file is formatted as expected, with extraneous whitespace removed:

image

Note

I attempted to try installing WPGraphQL CS from here using composer require --dev axepress/wp-graphql-cs to see if we could use a single source of rulesets, but I ran into this error:

image

I'm not sure if it's worth pursuing to use the single code rule source, or if it's a rabbit hole I shouldn't venture down right now. Open to ideas. Thanks!

Copy link

@justlevine justlevine left a comment

Choose a reason for hiding this comment

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

LGTM

@jasonbahl
Copy link
Collaborator

@missionmike looks like tests are failing, but not related to your PR. I opened another PR #292 to address the failing tests (needed to update to use docker compose instead of docker-compose)

@jasonbahl
Copy link
Collaborator

@missionmike I merged the other PR that fixed the docker tests and just merged the changes into this PR so tests should run now

@jasonbahl jasonbahl assigned jasonbahl and unassigned jasonbahl Aug 27, 2024
@jasonbahl jasonbahl self-requested a review August 27, 2024 17:28
@josephfusco josephfusco merged commit 7300dce into wp-graphql:main Aug 29, 2024
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants