WordPress.org

Make WordPress Core

Opened 3 months ago

Closed 5 weeks ago

Last modified 9 days ago

#47853 closed enhancement (fixed)

Minor update to composer lint scripts

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

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 3 months ago.
Update composer.json
47853.2.diff (806 bytes) - added by dingo_d 3 months ago.
Fix the review comments
47853.3.diff (844 bytes) - added by desrosj 5 weeks ago.

Download all attachments as: .zip

Change History (11)

@dingo_d
3 months ago

Update composer.json

#1 @jrf
3 months 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
3 months ago

  • Component changed from General to Build/Test Tools

#3 @netweb
3 months ago

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

@dingo_d
3 months ago

Fix the review comments

#4 @dingo_d
3 months ago

  • Keywords needs-refresh removed

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

#5 @desrosj
5 weeks ago

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

@desrosj
5 weeks ago

#6 @desrosj
5 weeks 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
5 weeks 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
9 days 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.