-
Notifications
You must be signed in to change notification settings - Fork 1
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
Add composer and PHPCS for local linting #127
Conversation
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.
Added a few notes inline, mainly to make sure we ignore a few more things.
Thank you for the CR, @peterwilsoncc, and my apologies for the late reply and tardiness of some of the code: I've added both your suggestions and resolved the conflict with the most recent changes in the |
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.
@cameronterry No need to apologise, we're all very busy. Thanks for the PR.
This LGMT
@peterwilsoncc I think I've resolved those blocking required actions, but looks like composer files have conflicts to resolve before merging. |
# Conflicts: # composer.json # composer.lock
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.
@jeffpaul Conflicts have been resolved, tests are passing.
Description of the Change
Cleans up the NPM scripts and adds PHPCS through Composer, enabling linting for PHP in addition to CSS and JS; my assumption being this would be useful longer term than removing the Composer references.
I've removed the
composer install --no-dev -o &&
portion fromnpm run build-release
but kept the other references innpm run lint
andnpm run lint-release
, as it has PHPCS will now be available.Running
npm run lint-php
orcomposer run lint
returns:Closes #116
How to test the Change
npm install
.npm run build-release
.npm run lint-php
.npm run lint
(this seemingly runs into an issue seen in one of the build actions, https://github.com/10up/insecure-content-warning/actions/runs/5336668720/jobs/9671677686, which I think is related to this, Update node and NPM versions #114?)Changelog Entry
Credits
Props @cameronterry
Checklist: