-
Notifications
You must be signed in to change notification settings - Fork 13
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 #35 | Upgrade WordPress Coding Standards to 3.1.0 #49
base: develop
Are you sure you want to change the base?
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.
Hi @sksaju,
Thanks for the working on the PR. I have started testing it, but getting /action/entrypoint.sh: line 5: composer: not found
error. Could you please help with checking it once?
You can find logs of action run here: https://github.com/iamdharmesh/autoshare-for-twitter/actions/runs/10319895557/job/28569247554?pr=5
Thank you.
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.
Looks good to me!
5d26790
to
85891aa
Compare
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.
Ensure that the composer-installed WPCS is available to all standards, and add PHPCSUtils to the default WPCS run.
Co-authored-by: Ben Keith <[email protected]>
Co-authored-by: Ben Keith <[email protected]>
Co-authored-by: Ben Keith <[email protected]>
Install missing sniff in config path Update missing sniff in config path
5b9c999
to
5b45d2b
Compare
@benlk I made some changes but it seems to be failing https://github.com/thrijith/autoshare-for-twitter/actions/runs/10938472447/job/31076394106?pr=2#step:4:87 The error is |
entrypoint.sh
Outdated
decide_all_files_or_changed "${HOME}/wpcs" | ||
echo "Setting up default WPCS" | ||
git clone --depth 1 --branch 1.0.11 https://github.com/PHPCSStandards/PHPCSUtils ${HOME}/phpcsutils | ||
decide_all_files_or_changed "$(composer config home)/vendor/wp-coding-standards/wpcs,${HOME}/phpcsutils/PHPCSUtils" |
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.
Hi @sksaju, when using the default standard, I'm getting ERROR: Referenced sniff "Universal.NamingConventions.NoReservedKeywordParameterNames" does not exist
. This occurs because PHPCSExtra's path isn't included in PHPCS's installed_paths
configuration, even though it's installed globally via composer.
I think we need to add PHPCSExtra's path to the decide_all_files_or_changed
call:
decide_all_files_or_changed "$(composer config home)/vendor/wp-coding-standards/wpcs,${HOME}/phpcsutils/PHPCSUtils,$(composer config home)/vendor/phpcsstandards/phpcsextra"
entrypoint.sh
Outdated
@@ -105,7 +114,7 @@ if [ "${INPUT_STANDARD}" = "WordPress-VIP-Go" ] || [ "${INPUT_STANDARD}" = "Word | |||
git clone --depth 1 -b 2.3.3 https://github.com/Automattic/VIP-Coding-Standards.git ${HOME}/vipcs | |||
git clone https://github.com/sirbrillig/phpcs-variable-analysis ${HOME}/variable-analysis | |||
|
|||
decide_all_files_or_changed "${HOME}/wpcs,${HOME}/vipcs,${HOME}/variable-analysis" | |||
decide_all_files_or_changed "$(composer config home)/vendor/wp-coding-standards/wpcs,${HOME}/vipcs,${HOME}/variable-analysis" |
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.
Hi @sksaju, when using the WordPress-VIP-Go standard, I'm getting ERROR: Referenced sniff "WordPress.PHP.StrictComparisons" does not exist
. This occurs because the action is missing the PHPCSUtils path in its configuration.
I think we need to add PHPCSUtils to the decide_all_files_or_changed
paths:
decide_all_files_or_changed "$(composer config home)/vendor/wp-coding-standards/wpcs,${HOME}/vipcs,${HOME}/variable-analysis,${HOME}/phpcsutils/PHPCSUtils"
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.
@s3rgiosan I've updated the PR, could you take another look, please? Thank you
Description of the Change
Closes #35
How to test the Change
Changelog Entry
Credits
Props @username, @username2, ...
Checklist: