Add composer and PHPCS for local linting #127
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
peterwilsoncc
left a comment
peterwilsoncc
left a comment
There was a problem hiding this comment.
Added a few notes inline, mainly to make sure we ignore a few more things.
github-actions
bot
added
the
needs:refresh
github-actions
bot
added
needs:code-review
Author
|
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 |
peterwilsoncc
previously approved these changes
Sep 18, 2023
Contributor
peterwilsoncc
left a comment
peterwilsoncc
left a comment
There was a problem hiding this comment.
@cameronterry No need to apologise, we're all very busy. Thanks for the PR.
This LGMT
Contributor
github-actions
bot
added
the
needs:refresh
Member
|
@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
github-actions
bot
removed
the
needs:refresh
peterwilsoncc
approved these changes
Sep 20, 2023
Contributor
peterwilsoncc
left a comment
peterwilsoncc
left a comment
There was a problem hiding this comment.
@jeffpaul Conflicts have been resolved, tests are passing.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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-releasebut kept the other references innpm run lintandnpm run lint-release, as it has PHPCS will now be available.Running
npm run lint-phporcomposer run lintreturns: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: