[wp-trac] [WordPress Trac] #41057: Update PHP codebase per WordPress PHP Coding Standards

[WordPress Trac]

#41057: Update PHP codebase per WordPress PHP Coding Standards
----------------------------+-----------------------
 Reporter:  netweb          |       Owner:  pento
     Type:  task (blessed)  |      Status:  accepted
 Priority:  normal          |   Milestone:  4.9
Component:  General         |     Version:
 Severity:  normal          |  Resolution:
 Keywords:                  |     Focuses:
----------------------------+-----------------------

Comment (by netweb):

 Replying to [comment:21 jrf]:
 > @netweb Thanks for the ping.

 No problem, and thank you @jrf for the momentous contributions to WPCS

 > Just a couple of notes:
 >
 > * Please make sure you use at least PHPCS 2.9.0 or 2.9.1 when generating
 the patch files. These versions contain a fix for an issue with tabs vs
 spaces to indent lines which could be relevant. See
 [https://github.com/squizlabs/PHP_CodeSniffer/pull/1404 PHPCS PR 1404].
 >  Those versions also contain numerous other fixes which make the sniffs
 WPCS uses from upstream as well as some WPCS sniffs which use token arrays
 as defined in PHPCS, better, so could yield better results.
 >
 > * Yes, please use the current WPCS `develop` branch as it contains two
 notable new sniffs which are likely to be relevant: as mentioned before
 [https://github.com/WordPress-Coding-Standards/WordPress-Coding-
 Standards/pull/941 WPCS PR 941] for the array indentation, but also
 [https://github.com/WordPress-Coding-Standards/WordPress-Coding-
 Standards/pull/942 WPCS PR 942] which checks that spaces are used for mid-
 line alignment.
 >
 > * I'd strongly advise visual verification of the generated patches. I
 already noticed some fixes which don't look right in the above attached
 patches - for instance in `src/wp-content/themes/twentyeleven/404.php` -.
 As I'm unclear what versions of PHPCS has been used to generated these
 patches, these issues may have been addressed already, but that would need
 to be verified.
 >  Please report any bugs you find to [https://github.com/WordPress-
 Coding-Standards/WordPress-Coding-Standards/issues WPCS] and I'll / we'll
 try to address them as soon as possible.

 The patches I attached here were generated using PHPCS `2.9.1` and WPCS
 `0.11.0`, we've now switched to using WPCS `develop` which addresses the
 nested arrays issue you noticed in Twenty Eleven's `404.php`

 Using WPCS `develop` a few files are no longer fixable on my local setup
 that were ''fixed'' when using WPCS `0.11.0`, currently the two instances
 of this is for `src/wp-content/themes/twentyeleven/404.php` and `src/wp-
 content/themes/twentyeleven/functions.php` files. I'll go through and
 audit this list of files later today

 > > The commit here should include a `phpcs.xml` that does the requisite
 exclusions for 3rd-party PHP, as well as declaring that the WordPress-Core
 standard is being used.

 For PHPUnit we use the `.dist` configuration file, likewise we should use
 PHPCS' `phpcs.xml.dist` to allow developers to define/test any alternate
 configurations locally.

 > * You may want to consider adding the `WordPress-Docs` ruleset as well.
 This ruleset is not complete yet, but what it does cover is in the Core
 PHP Documentation Handbook page and a number of sniffs included there also
 include auto-fixers.

 Let's '''not''' add this at this stage, the PHP Docs can be modified at
 anytime, code churn is not an issue for the docs.

 > > Should we also update `.travis.yml` to start running PHPCS as well?
 >
 > * Not all issues which will be identified can be auto-fixed. As this
 ticket - so far - only contains patches for things which can be auto-
 fixed, there will probably still be a list of violations after these
 patches have gone in. Those would need to be addressed before activating
 an SVN commit hook / travis build script change, otherwise all builds will
 fail / commits be rejected.
 >
 > * Protip: please run PHPCS only ''once'' per build preferably against a
 reasonably high PHP version (PHP 5.5 or up). All sniffs are unit tested
 against all PHP versions, so running PHPCS against each build would just
 return the same results each time. For an example setup, see:
 https://github.com/jrfnl/make-phpcs-work-for-you/tree/master/travis-
 examples

 Initially we can add a single new job to Travis CI that is "allowed to
 fail" using PHP 7.0

 > * Side-note: PHPCS 3.x contains the ability to create filters and comes
 with a build-in filter `gitmodified`. While this is not that useful while
 WP development is still using SVN, this may be good to keep in mind for
 if/when development would move to a Git-based system. The filter causes
 PHPCS to only report on violations found in newly added and changed files
 within a commit.

 Excellent, we can still utilise this as part of #34694

 > > Can we get a (grunt) pre commit hook on the svn server to block
 commits that don't have the proper format?
 >
 > * The WPCS repository contains an example for a pre-commit hook which
 might be useful as inspiration for this: https://github.com/WordPress-
 Coding-Standards/WordPress-Coding-Standards/blob/develop/bin/pre-commit

 As there are core committers using SVN and Git via `git svn dcommit` to
 commit a pre-commit hook should this into consideration, and hopefully
 support both methods

--
Ticket URL: <https://core.trac.wordpress.org/ticket/41057#comment:22>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform