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

Format existing code files and exclude playground/vendor #163

Merged
merged 6 commits into from Feb 21, 2024

Conversation

sejas
Copy link
Collaborator

@sejas sejas commented Feb 20, 2024

What?

On #154 we added the playground plugin with multiple vendor files that npx nx format was formatting after each push.

Captura de pantalla 2024-02-20 a las 15 09 46

This PR format existing code files and excludes those in vendor.

  • Format code files.
  • Excludes packages/vendor/* files to be formatted
  • Excludes .nx/cahce from git
  • Moves package/playground/phpcs.xml to the root directory

Why?

It's better to keep consistency with formatting tools.

How?

I executed npx nx format.

Testing Instructions

  • Tests should pass

@sejas sejas changed the title Format code Format existing code files and exclude playground/vendor Feb 20, 2024
@sejas sejas self-assigned this Feb 20, 2024
Copy link
Member

@danielbachhuber danielbachhuber left a comment

Choose a reason for hiding this comment

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

I was running into this when working on #161 this morning.

Is there a reason we don't run a linter in CI?

Copy link
Collaborator

@bgrgicak bgrgicak left a comment

Choose a reason for hiding this comment

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

Thanks @sejas!

@bgrgicak
Copy link
Collaborator

@sejas should we move the packages/playground/phpcs.xml file to the root folder?

@sejas
Copy link
Collaborator Author

sejas commented Feb 21, 2024

@sejas should we move the packages/playground/phpcs.xml file to the root folder?

Yes, I think it makes sense to have a unique phpcs.xml at the root directory. I moved it here 7073a53
Although, I tried to make it work and phpcs is returning. ERROR: No sniffs were registered.
When I used the whole WordPress phpcs.xml it worked well. We should check if we need to modify the phpcs configuration or the issue only happens on my end.

Is there a reason we don't run a linter in CI?

The project is not 100% well configured and the linter doesn't throw any error, but prettier format the files.
You can try it by breaking the format in any .js file and then running npm run lint, you will see 0 errors, but npx nx format will format the files.
I'll need more time to fix the linter to make it work properly and then, yes! it would be great to add it to CI GH Actions.
Another possible workaround would be adding husky pre-push hook to format the files.

I also added .nx/cahce to .gitignore 0817071. These files weren't created before.

@sejas sejas merged commit 4233988 into trunk Feb 21, 2024
2 checks passed
@sejas sejas deleted the update/format-code branch February 21, 2024 12:30
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.

None yet

3 participants