Make WordPress Core

Opened 3 years ago

Last modified 3 years ago

#54064 new defect (bug)

Add phpcs run command to Docker

Reported by: pbearne's profile pbearne 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

#2 @pbearne
3 years ago

Used the composer pattern from #53945

This ticket was mentioned in Slack in #core-test by pbearne. View the logs.


3 years ago

#4 @jrf
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 @jrf
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 @pbearne
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

Note: See TracTickets for help on using tickets.