[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 jrf):

 @netweb Thanks for the ping.

 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 try to address
 them as soon as possible.

 > 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.

 * 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.

 > If we need exceptions, we can mark that in the code with comments. see
 https://github.com/squizlabs/PHP_CodeSniffer/wiki/Advanced-Usage#ignoring-
 parts-of-a-file

 * I'd strongly advise to use these type of comments as sparingly as
 possible. This will turn PHPCS off completely for those lines, not just
 for the one sniff which may be problematic.

 * As a side-note to this: PHPCS 3.x (with which WPCS is not - yet -
 compatible) contains a command line option `--ignore-annotations` to
 ignore any such annotations. This will be useful in the future to review
 from time to time if any such annotations are used and if so, if these are
 still needed.

 > 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

 * 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.


 > There's a ticket for fixing the ruleset.

 * And a PR has been created & merged since, so the "long conditional end
 comments" sniff has effectively been removed from WPCS `develop`.

 > 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

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