stevegrunwell / wp-enforcer Goto Github PK

The README currently suggests adding the following "commands" to the package manifest, but it doesn't indicate where they should land:

"post-install-cmd": [
	"wp-enforcer"
],
"post-update-cmd": [
	"wp-enforcer"
]

As a total n00b using composer I thought these were some kind of built-in. It wasn't until later I realized I should've added them to the scripts section and not in the root of the manifest. I imagine a quick GH search for apps using this package will turn up the same.

As we start introducing options (see #6, #7) a "help" option (-h and --help) need to be implemented so people can see the available options from the CLI before executing the installation script.

Bash is awesome ... unless you're on a PC ... :-P

https://github.com/stevegrunwell/wp-enforcer/blob/develop/bin/hooks/pre-commit#L9

This line is hard-coded and does not respect the -c flag of the CLI.

# Determine if we have a phpcs.xml file
if [[ -f ./phpcs.xml ]]; then
	standard="--standard=./phpcs.xml"
else
	standard=''
fi

The above needs to be updated to accept an arg when -c is set in the CLI.

http://stackoverflow.com/a/44083470/712334

I added a few problems in my code to see if it works, but I am commiting code and nothing happens.
I am using Mac, testing a plugin and I have installed it just the way you wrote and added extra config in the composer.json.

What could be wrong?

I'm considering rewriting the main wp-enforcer Bash script in PHP. It would still be run the same way, but rather than dealing with a very procedural scripting language we could a) leverage the PHP CLI Tools package maintained by the WP-CLI team, b) make testing the code for this [very WordPress-specific tool] way easier (see #16), and generally make the script more portable (considering the package is installed via Composer and thus only available in environments that can already execute code via the PHP CLI).

Anyone want to either talk me off that ledge or convince me to take the plunge before we hit a 1.0.0?

Cc: @bswatson @daveross @ericmann

In addition to --vip, wp-enforcer should support custom rulesets.

One of my clients has a project.ruleset.xml file in the root directory that our team should be using on their project.

Maybe a --customruleset='...' option?

PHP Code Beautifier and Fixer can be used to automatically fix many common or minor issues and would be great as an additional option.

Possibly related to #12, when I have WP Enforcer enabled, I'm prevented from adding images to my Git repo. PHPCS reports multiple errors, most noticeably "mixed line endings" and "ASP style opening tags" in the files. I have theInternal.NoCodeFound rule added but unless I also add an exclude-pattern for those files it still tries to check them, which is super frustrating.

This happens in my theme with screenshot.png and some JPEG images bundled with it.

I would love to see a cloud function that can do the following:

• Verify the repo is using latest version of WP Enforcer, maybe auto update it.
• Submit a automatic pull request of phpcbf changes. I would see it working similar to how imgbot works.

As @johnpbloch pointed out, ./vendor/bin automatically gets prepended to the user's $PATH, so the hard-coded values are not only unnecessary but could actually impact portability (since the vendor directory can be moved around).

WordPress has an official .jshintrc file and PHP_CodeSniffer can be configured to run it automatically, we just need to put those pieces together so JavaScript can automatically be linted/hinted/otherwise checked.

I mentioned it in a tweet to @tarendai this evening, so I'd might as well add it here:

Considering PHP_CodeSniffer supports the VIP Coding Standards, it wouldn't take much effort to create a branch that uses those standards. Alternately, a --vip option during installation could make WP Enforcer helpful for VIP projects.

Out-of-the-box the plugin won't let me commit the changes to add it to my repo. It looks like it's running PHPCS over everything, not just PHP files. I'm not sure whether the desired solution here would just be to add those files (Which should be common to anyone using this?) to the same phpcs.xml as excluded paths? or to make PHPCS only called on PHP files? Or is there something funky going on for me?

$ git commit -m 'Trialling WP enforcer for pre-commit awesomeness'
Running PHP CodeSniffer...

FILE: ...t/.gitignore
--------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
--------------------------------------------------------------------------------
 1 | WARNING | No PHP code was found in this file and short open tags are not
   |         | allowed by this install of PHP. This file may be using short
   |         | open tags but PHP does not allow them.
--------------------------------------------------------------------------------

FILE: ...t/composer.json
--------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
--------------------------------------------------------------------------------
 1 | WARNING | No PHP code was found in this file and short open tags are not
   |         | allowed by this install of PHP. This file may be using short
   |         | open tags but PHP does not allow them.
--------------------------------------------------------------------------------

FILE: ...t/composer.lock
--------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
--------------------------------------------------------------------------------
 1 | WARNING | No PHP code was found in this file and short open tags are not
   |         | allowed by this install of PHP. This file may be using short
   |         | open tags but PHP does not allow them.
--------------------------------------------------------------------------------

FILE: ...t/phpcs.xml
--------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
--------------------------------------------------------------------------------
 1 | WARNING | No PHP code was found in this file and short open tags are not
   |         | allowed by this install of PHP. This file may be using short
   |         | open tags but PHP does not allow them.
--------------------------------------------------------------------------------

Time: 17ms; Memory: 4Mb

Problem

In version 0.5.0, when I run wp-enforcer it removes the standards already set for a project.

Description

I have multiple coding standards installed in my project (WP Coding Standards and PHPCompatibility) and a script to set them for use with the project's PHPCS. When I run wp-enforcer, it only checks for wp-coding-standards/wpcs and removes the other standard on every run, which causes checks to fail because PHPCS can't find the expected sniffs.

Possible Resolution

It would be nice for WP Enforcer to honor the existing config and not remove the standards set, either by not forcing the standards inside the script or by appending wpcs to the installed_paths only if it's not present already.

cp ~/test/.git/hooks/pre-commit: No such file or directory
chmod ~/test/.git/hooks/pre-commit: No such file or directory
Copying pre-commit hook

WP Enforcer installed successfully!

Installation of hooks failed to install, but still reported a success.

Good recommendation from @bswatson:

When the Git hook runs it should only be run on files that have changed. This will reduce friction when implementing into an existing codebase as a file won't get checked (or at least not flagged) until something inside it has changed. Developers can still run ./vendor/bin/phpcs to scan the entire codebase, but this fix will make WP Enforcer much less of a "get everything in order before this is useful" tool.

When running the install script on OS X 10.11.1 I get no output from the shell script. It also replaces the pre-commit hook file entirely without asking.

Here is a debug output of the script:

+ readonly VERSION=0.4.0
+ VERSION=0.4.0
+++ dirname ./vendor/bin/wp-enforcer
++ cd ./vendor/bin
++ pwd -P
+ readonly BIN=/Users/brandoncamenisch/Sites/Vagrants/vvv/www/ten/wordpress/wp-content/plugins/vehicle-data/vendor/bin
+ BIN=/Users/brandoncamenisch/Sites/Vagrants/vvv/www/ten/wordpress/wp-content/plugins/vehicle-data/vendor/bin
+++ readlink ./vendor/bin/wp-enforcer
++ dirname ../stevegrunwell/wp-enforcer/bin/wp-enforcer
+ readonly DIR=/Users/brandoncamenisch/Sites/Vagrants/vvv/www/ten/wordpress/wp-content/plugins/vehicle-data/vendor/bin/../stevegrunwell/wp-enforcer/bin
+ DIR=/Users/brandoncamenisch/Sites/Vagrants/vvv/www/ten/wordpress/wp-content/plugins/vehicle-data/vendor/bin/../stevegrunwell/wp-enforcer/bin
++ pwd
+ readonly PROJECT=/Users/brandoncamenisch/Sites/Vagrants/vvv/www/ten/wordpress/wp-content/plugins/vehicle-data
+ PROJECT=/Users/brandoncamenisch/Sites/Vagrants/vvv/www/ten/wordpress/wp-content/plugins/vehicle-data
+ vip=false
++ getopt -o h,v -l help,version,vip --
+ options=' -- h,v -l help,version,vip --'
+ '[' 0 -gt 0 ']'
+ echo 'Installing WP Enforcer...'
Installing WP Enforcer...
+ [[ ! -d /Users/brandoncamenisch/Sites/Vagrants/vvv/www/ten/wordpress/wp-content/plugins/vehicle-data/.git ]]
+ [[ ! -d /Users/brandoncamenisch/Sites/Vagrants/vvv/www/ten/wordpress/wp-content/plugins/vehicle-data/.git/hooks ]]
+ [[ -f /Users/brandoncamenisch/Sites/Vagrants/vvv/www/ten/wordpress/wp-content/plugins/vehicle-data/.git/hooks/pre-commit ]]
+ cmp /Users/brandoncamenisch/Sites/Vagrants/vvv/www/ten/wordpress/wp-content/plugins/vehicle-data/vendor/bin/../stevegrunwell/wp-enforcer/bin/hooks/pre-commit /Users/brandoncamenisch/Sites/Vagrants/vvv/www/ten/wordpress/wp-content/plugins/vehicle-data/.git/hooks/pre-commit
+ echo 'pre-commit hook already exists, skipping'
pre-commit hook already exists, skipping
+ [[ ! -f /Users/brandoncamenisch/Sites/Vagrants/vvv/www/ten/wordpress/wp-content/plugins/vehicle-data/phpcs.xml ]]
++ /Users/brandoncamenisch/Sites/Vagrants/vvv/www/ten/wordpress/wp-content/plugins/vehicle-data/vendor/bin/phpcs --config-show
++ grep installed_paths:
+ installed_paths='installed_paths: ../../wp-coding-standards/wpcs'
+ [[ installed_paths: ../../wp-coding-standards/wpcs != \i\n\s\t\a\l\l\e\d\_\p\a\t\h\s\:\ \.\.\/\.\.\/\w\p\-\c\o\d\i\n\g\-\s\t\a\n\d\a\r\d\s\/\w\p\c\s ]]
+ echo

+ echo 'WP Enforcer installed successfully!'
WP Enforcer installed successfully!
+ exit 0

Would it be possible to modify the hook to check only the code that was added in a given commit (instead of checking changed files as in #1)? I think this would reduce friction even more when using wp-enforcer into an existing codebase.

Consider it @bswatson's Christmas present 😉

Context:

• I have phpcs installed already (/usr/local/bin/phpcs).
• I have (an older version) of the WP coding standards installed (Which happens not to have WordPress-Docs)

When setting up with an out-of-the-box install, everything fails since the pre-commit hook is using my "normal" phpcs which knows nothing about the standards under vendor, specifically it knows nothing about WordPress-Docs

I can see the arguments both ways about global installs vs locals, but I think if we're taking the time to download the dependencies then we should set things up to use them. This would allow different projects to use different versions if required.

This would mean prefixing phpcs in the pre-commit hook with an explicit patch, and I think also providing a path for the standards to use, not just the name.

http://semver.org/#how-do-i-know-when-to-release-100

If your software is being used in production, it should probably already be 1.0.0. If you have a stable API on which users have come to depend, you should be 1.0.0. If you’re worrying a lot about backwards compatibility, you should probably already be 1.0.0.

If you'd like to require WP Enforcer for all developers on your project, you can add the following commands to your composer.json file to have WP Enforcer automatically set up Git hooks on composer install and composer update:

"post-install-cmd": [
	"wp-enforcer"
],
"post-update-cmd": [
	"wp-enforcer"
]

Please state the outcomes provided in performing this action, otherwise you may be proliferating the disease known as copypasta.

Install docs say to run:

composer require stevegrunwell/wp-enforcer

But wouldn't it be more appropriate to composer require stevegrunwell/wp-enforcer --dev?

As the project is picking up traction I'm regretting not learning how to unit test Bash scripts earlier. While the scripts may be more integration tests than pure unit tests, I'd love to get some coverage on our various options and features.

Currently composer.json has a hard dependency on WPCS, and is using 0.10.0. Because they're not at a 1.0 and the current rule is ^ this change will not be automatic for minor updates (only one a major lands will that be true). As a result, we need to update to the new standard manually.