Skip to content
This repository has been archived by the owner on Sep 16, 2019. It is now read-only.

Update CS setup #1399

Open
jrfnl opened this issue Sep 1, 2019 · 0 comments
Open

Update CS setup #1399

jrfnl opened this issue Sep 1, 2019 · 0 comments

Comments

@jrfnl
Copy link

jrfnl commented Sep 1, 2019

The current code style and PHP compatibility setup is pretty out-of-date, inconsistent and convoluted:

  • The Travis script seems to try use your theme to unit test PHPCompatibility.
    Honestly, you don't have to as PHPCompatibility does extensive unit testing itself.
    Just run it once on a high PHP version with a properly set testVersion and you're done - or in case you want to test separately for compatibility with PHP 5.6 and above and compatibility with PHP 5.2/5.3-5.5, run it twice, but running it the way it's done now.... is really not very effective.
    See: https://github.com/PHPCompatibility/PHPCompatibility#sniffing-your-code-for-compatibility-with-specific-php-versions
  • As this is a WordPress theme, you probably should use PHPCompatibilityWP instead of PHPCompatibility.
    See: https://github.com/PHPCompatibility/PHPCompatibilityWP
  • The Composer file is pointing to the abandoned wimg/php-compatibility repo.
    The package was moved to phpcompatibility/php-compatibility a year ago....
  • squizlabs/php_codesniffer is not a direct dependency for your theme, but a dependency of PHPCompatibility(WP). Don't require it, but let the package which depends on it manage the supported versions.
  • The DealerDirect PHPCS Composer plugin has released a new version and should be updated to use ^0.5.0.
    Composer treats minors < 1.0 as majors, so this needs to be done explicitly.
    See: https://github.com/Dealerdirect/phpcodesniffer-composer-installer/releases
  • You seem to include a codesniffer.ruleset.xml file, but don't require-dev the needed dependency (WPCS) for it.
  • The WPCS version which the ruleset seems to be based on is very much out of date and a number of sniffs have been renamed since. This ruleset will not work anymore.
  • The ruleset has a non-standard name which means that PHPCS will not pick up on it automatically.
    Please consider renaming the file to phpcs.xml.dist.
  • The code is not actually checked against the ruleset via Travis anyway, so what is going on here ?
  • Please consider using the WP ThemeReview ruleset: https://github.com/WPTRT/WPThemeReview

If this is truly intended as a starter-theme, I would strongly suggest leading by example and setting these things up properly and correctly.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant