Opened 3 years ago
Last modified 3 years ago
#54064 new defect (bug)
Add phpcs run command to Docker
Reported by: |
|
Owned by: | |
---|---|---|---|
Milestone: | Awaiting Review | Priority: | normal |
Severity: | normal | Version: | |
Component: | Build/Test Tools | Keywords: | has-patch |
Focuses: | Cc: |
Description
I keep on getting formatting error in githib actions but have no easy way to see what they are so have added phpcs commands to the Docker build
this runs phpcs on the tests folder
npm run phpcs:tests
This runs phpcs in the src folder
npm run phpcs:src
Change History (6)
This ticket was mentioned in PR #1653 on WordPress/wordpress-develop by pbearne.
3 years ago
#1
- Keywords has-patch added
This ticket was mentioned in Slack in #core-test by pbearne. View the logs.
3 years ago
#4
@
3 years ago
@pbearne While I don't think it's a bad idea to add these commands, this doesn't directly address the problem you highlighted: that the GH actions output of the PHPCS run indecipherable (because it is formatted for display in a PR via the cs2pr tool).
To address that second point, I'd like to suggest also fixing the commands used in the GH actions workflows as per the second code sample for PHPCS in the cs2pr readme.
For local use, running the commands as proposed to be added to the Docker scripts is highly inefficient (slow) as PHPCS will scan the whole project.
For most users, it may be a good idea to educate them about PHPCS better (though I don't know exactly where to do this).
If you want to run these commands in Docker, that means you will have already run composer install or
composer update` locally.
In that case, you don't need to be in the Docker container to run a local PHPCS check as you can run the existing Composer scripts: composer compat
, composer lint
, composer lint:errors
or composer format
.
Better yet, call vendor/bin/phpcs
directly using --filter=GitStaged
(= staged files) or --filter=GitModified
(= unstaged files) to get PHPCS to scan just and only the files which have been changed, but not committed yet.
As a side-note, it would probably be a good idea to use the same script names and scripts in the Docker config as are used in the Composer config for consistency and probably even better for Docker to call the Composer scripts under the hood as otherwise the chances are high that at some point one of the scripts will be adjusted in one, but not the other file and they will diverge, making the outcome even more unpredictable across workflows.
#5
@
3 years ago
Last note: the pattern from #53945 is still under discussion as it isn't stable across PHP versions, so this ticket should probably not use it (yet).
#6
@
3 years ago
When I got cs2pr working I found it outputed HTML which in CLI doesn't make sense the default output is OK for the use case
Yes it would be nice to be able to pass file names but this is not an easy option with PHPCS all we can pass it is path/file
I can see myself do this in a shell
Trac ticket: https://core.trac.wordpress.org/ticket/54064#ticket