WPTT / WPThemeReview

PHP_CodeSniffer rules (sniffs) to enforce WordPress theme review coding conventions
MIT License
208 stars 38 forks source link

Remove aliases reference from autoload #196

Closed dingo-d closed 5 years ago

dingo-d commented 5 years ago

I was trying to set up a test for the develop branch so that I can do a step by step instructions on the make blog and there was an error coming from the autoload.php.

Steps to reproduce

  1. Go to theme folder and run
composer require --dev wptrt/wpthemereview:dev-develop dealerdirect/phpcodesniffer-composer-installer:^0.4.4

This installs the latest version:

./composer.json has been created
Loading composer repositories with package information
Updating dependencies (including require-dev)
Package operations: 7 installs, 0 updates, 0 removals
  - Installing squizlabs/php_codesniffer (3.4.0): Downloading (100%)
  - Installing dealerdirect/phpcodesniffer-composer-installer (v0.4.4): Downloading (100%)
  - Installing wp-coding-standards/wpcs (2.0.0): Downloading (100%)
  - Installing phpcompatibility/php-compatibility (9.1.1): Downloading (100%)
  - Installing phpcompatibility/phpcompatibility-paragonie (1.0.1): Downloading (100%)
  - Installing phpcompatibility/phpcompatibility-wp (2.0.0): Downloading (100%)
  - Installing wptrt/wpthemereview (dev-develop 895ed6d): Cloning 895ed6d2ed from cache
dealerdirect/phpcodesniffer-composer-installer suggests installing dealerdirect/qa-tools (All the PHP QA tools you'll need)
phpcompatibility/php-compatibility suggests installing roave/security-advisories (dev-master || Helps prevent installing dependencies with known security issues.)
phpcompatibility/phpcompatibility-paragonie suggests installing roave/security-advisories (dev-master || Helps prevent installing dependencies with known security issues.)
phpcompatibility/phpcompatibility-wp suggests installing roave/security-advisories (dev-master || Helps prevent installing dependencies with known security issues.)
Writing lock file
Generating autoload files
  1. Run
vendor/bin/phpcs -i

Which gives out:

The installed coding standards are PEAR, Zend, PSR2, MySource, Squiz, PSR1, PSR12, PHPCompatibility, PHPCompatibilityParagonieRandomCompat, PHPCompatibilityParagonieSodiumCompat, PHPCompatibilityWP, WordPress, WordPress-Extra, WordPress-Docs, WordPress-Core and WPThemeReview

Run

vendor/bin/phpcs -p . --standard=WPThemeReview

Which gives the error

Uh oh... can't find WPCS.

If you use Composer, please run `composer install`.
Otherwise, make sure you set a `WPCS_DIR` environment variable
pointing to the WPCS directory.

When I tried the same with the older version (without specifying a branch), all worked fine.

In the autoload.php the

file_exists( $wpcsDir . $ds . 'WordPress' . $ds . 'PHPCSAliases.php' );

Will return false which causes the error. Seeing how the Aliases were removed, the entire

// Try and load the WPCS class aliases file.
if ( false !== $wpcsDir && file_exists( $wpcsDir . $ds . 'WordPress' . $ds . 'PHPCSAliases.php' ) ) {
    require_once $wpcsDir . $ds . 'WordPress' . $ds . 'PHPCSAliases.php';
} else {
    echo 'Uh oh... can\'t find WPCS.

If you use Composer, please run `composer install`.
Otherwise, make sure you set a `WPCS_DIR` environment variable
pointing to the WPCS directory.
';

    die( 1 );
}

block needs to be removed.

dingo-d commented 5 years ago

@jrfnl Could you look if this is ok, so that I can publish a make post on the theme review make blog? 🙂

jrfnl commented 5 years ago

Oh and if it all works as expected once the file is dropped + the autoload command removed from the ruleset, the unit test instructions in the CONTRIBUTING.MD file can also be shortened a lot.

dingo-d commented 5 years ago

Ok I'll fix this, test and update the PR :+1:

@jrfnl Which parts should I remove from the CONTRIBUTING.md file? The entire Other setups part? That part is the only one containing the reference to WPCS_DIR which was used in autoload.php

dingo-d commented 5 years ago

I have change the contributing documentation, is this ok to merge @jrfnl ?

jrfnl commented 5 years ago

@dingo-d You have removed a little too much now. The Contributing guidelines should still explain how to run the unit tests with a git clone based setup.

dingo-d commented 5 years ago

I've made the changes, not sure if I added the correct parts, can you check if it's ok now? Thanks!

dingo-d commented 5 years ago

Added the fixes 🙂 Thanks for all the help @jrfnl

dingo-d commented 5 years ago

Go ahead and squash merge, better not to risk me messing something up 😄