Make WordPress Core

Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#47853 closed enhancement (fixed)

Minor update to composer lint scripts

Reported by: dingo_d's profile dingo_d Owned by: desrosj's profile desrosj
Milestone: 5.3 Priority: normal
Severity: normal Version: 5.3
Component: Build/Test Tools Keywords: has-patch has-dev-note
Focuses: coding-standards Cc:

Description

I've modified the composer scripts used for checking the standards of the core.

I've added @php prefix, which will make sure that the scripts are run against the same PHP version as with which Composer is called.
I've also changed the location from where the scripts are called, since it's assumed that the user will install the packages using Composer (this is the ./vendor/bin/phpcs and ./vendor/bin/phpcbf part).

Also the last script was just repeating the lint one so I've just used that.

I don't think that the code sniffer is used without composer so this should be ok.

Tested it locally and got the same result.

Attachments (3)

47853.diff (772 bytes) - added by dingo_d 6 years ago.
Update composer.json
47853.2.diff (806 bytes) - added by dingo_d 6 years ago.
Fix the review comments
47853.3.diff (844 bytes) - added by desrosj 6 years ago.

Download all attachments as: .zip

Change History (11)

@dingo_d
6 years ago

Update composer.json

#1 @jrf
6 years ago

@dingo_d Bit of feedback:

  1. @php ./vendor/bin/phpcs and the likes won't work on Windows. You need to reference the actual PHP file, not the bash script for it to work cross-platform, i.e. @php ./vendor/squizlabs/php_codesniffer/bin/phpcs. See: https://github.com/composer/composer/issues/7645
  2. --report-summary --report-source can be shortened to --report=summary,source (PHPCS 3.x feature)

Regarding removing --standard=phpcs.xml.dist 👍👍

PHPCS will automatically pick up on a phpcs.xml.dist file when it's available and not hard-coding it allows for devs to use the scripts and still overrule the config with a local phpcs.xml file containing extra rules (like PHPCompatibilityWP for instance).

#2 @SergeyBiryukov
6 years ago

  • Component changed from General to Build/Test Tools

#3 @netweb
6 years ago

  • Keywords needs-refresh added
  • Milestone changed from Awaiting Review to 5.3

@dingo_d
6 years ago

Fix the review comments

#4 @dingo_d
6 years ago

  • Keywords needs-refresh removed

Thanks for the review :) I've fixed the issues and added a new patch.

#5 @desrosj
6 years ago

  • Owner changed from pento to desrosj
  • Status changed from assigned to reviewing

@desrosj
6 years ago

#6 @desrosj
6 years ago

  • Keywords commit added

Thanks @dingo_d! 47853.3.diff is just a refresh of 47853.2.diff because I could not get that to apply cleanly. Using @php came up on another ticket that will add a new command.

#7 @desrosj
6 years ago

  • Resolution set to fixed
  • Status changed from reviewing to closed

In 46187:

Build/Test Tools: Improvements to Composer scripts.

Prefixing a script command with @php ensures that the script runs with the same version of PHP that Composer is installed with (and not the system default).

This change also updates the phpcs and phpcbf commands to use the version of PHPCS installed by Composer.

The —standard is no longer explicitly passed to the command. By default, PHPCS will look for phpcs.xml.dist, which is the name of the custom standards file currently in Core.

Props dingo_d, jrf.
Fixes #47853.

#8 @desrosj
6 years ago

  • Keywords has-dev-note added; commit removed

Mentioned in the Miscellaneous Developer Focused Changes dev note for 5.3: https://make.wordpress.org/core/2019/10/15/miscellaneous-developer-focused-changes-in-5-3/

Note: See TracTickets for help on using tickets.