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

YoastCS: check YoastCS native files for codestyle compliance #52

Merged
merged 2 commits into from
Dec 20, 2017

Conversation

jrfnl
Copy link
Collaborator

@jrfnl jrfnl commented Nov 30, 2017

Includes addition of a fully documented .phpcs.xml.dist file specifically for YoastCS itself and fixes to the IfElseDeclaration sniff to comply.

@jrfnl jrfnl added this to the 0.5.x milestone Nov 30, 2017
@jrfnl jrfnl requested a review from moorscode November 30, 2017 10:27
Copy link
Contributor

@moorscode moorscode left a comment

Choose a reason for hiding this comment

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

Possible small optimization change for cloning/checking out PHPCS.

@@ -42,12 +42,24 @@ before_install:
- export XMLLINT_INDENT=" "
- export PHPCS_DIR=/tmp/phpcs
- export PHPCS_BIN=$PHPCS_DIR/bin/phpcs
- export WPCS_DIR=/tmp/wpcs
- export PHPCOMPAT_DIR=/tmp/phpcompatibility
- mkdir -p $PHPCS_DIR && git clone --depth 1 https://github.com/squizlabs/PHP_CodeSniffer.git -b $PHPCS_BRANCH $PHPCS_DIR
- $PHPCS_BIN --config-set installed_paths $(pwd)
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this line still needed? It seems like the configuration is set twice when $SNIFF is configured.
When not sniffing, this does not need to be cloned or configured right?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

When not sniffing, the unit tests are run using the PHPCS unit test framework. For that to only test the YoastCS native sniffs, the YoastCS standard needs to be registered, so yes, this is still needed, though it could possibly be combined with the version of it below with an if/then/else syntax. You know how to do those in Travis scripts ?

.phpcs.xml.dist Outdated
<description>The Coding standard for the Yoast Coding Standard itself.</description>

<!-- Scan all files. -->
<file>.</file>
Copy link
Contributor

Choose a reason for hiding this comment

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

Indentation seems off.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed up.

Includes addition of a fully documented `.phpcs.xml.dist` file specifically for YoastCS itself.
@jrfnl jrfnl force-pushed the JRF/add-checking-for-codestyle branch from 1ac787f to f15ba0b Compare December 19, 2017 12:51
@moorscode moorscode merged commit acde5ab into master Dec 20, 2017
@moorscode moorscode deleted the JRF/add-checking-for-codestyle branch December 20, 2017 15:04
@moorscode moorscode modified the milestones: 0.5.x, 1.0.0, 0.5.0 Aug 24, 2018
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

2 participants