-
Notifications
You must be signed in to change notification settings - Fork 2
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
Clean up code to follow code standards #33
base: main
Are you sure you want to change the base?
Conversation
@@ -37,6 +37,18 @@ | |||
"require-dev": { | |||
"newfold-labs/wp-php-standards": "^1.2.3" | |||
}, | |||
"scripts": { | |||
"fix": [ | |||
"vendor/bin/phpcbf --standard=phpcs.xml ." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Composer scripts don't need vendor/bin
, it's naturally in $PATH for Composer.
@@ -37,6 +37,18 @@ | |||
"require-dev": { | |||
"newfold-labs/wp-php-standards": "^1.2.3" | |||
}, | |||
"scripts": { | |||
"fix": [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider calling it cs-fix
, per php-pds/composer-script-names, similarly below.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't feel very strongly on this. I like the idea of conventions but I don't particularly like the name.
Proposed changes
This adds
composer run lint
andcomposer run test
to run the code standards against our codebase & fixes up the current code to match it.Production
Development
Checklist
Further comments
There is definitely some more work that could be done here, along with some other cleanup and tests, but it's a good start.