Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add PHPCompatibilityWP Standard. #81

Merged
merged 10 commits into from
Jan 8, 2020
Merged

Add PHPCompatibilityWP Standard. #81

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
bbe0e3c
Add PHPCompatibilityWP standard. #74
tfrommen Aug 20, 2018
58e1d4d
Undo dependency update and sorting. That's for another PR.
tfrommen Aug 20, 2018
ff3bf52
Merge branch 'master' into php-compatibility
roborourke Mar 12, 2019
0da1b36
Merge branch 'master' into php-compatibility
mikeselander Apr 26, 2019
aa167c1
Updated CHANGELOG with change
mikeselander Apr 26, 2019
a968987
Bump PHPCompatibilityWP version
tfrommen May 1, 2019
27adbce
Bump minimum PHP version for compatibility checks
tfrommen May 1, 2019
af37b36
Merge branch 'master' into php-compatibility
mikeselander Dec 27, 2019
ba95fae
Merge branch 'master' into php-compatibility
mikeselander Jan 8, 2020
3945f6f
Remove empty CHANGELOG line
mikeselander Jan 8, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
## Unreleased (0.7.0)

### Added
- Added PHPCompatibilityWP standard to PHPCS #81

### Changed
- Exclude `load.php` from `NamespaceDirectoryNameSniff` #131
- Allow `json_encode` / `json_decode` function usage #97
Expand Down
4 changes: 4 additions & 0 deletions HM/ruleset.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@

<autoload>bootstrap.php</autoload>

<!-- Check for PHP cross-version compatibility. -->
<config name="testVersion" value="7.0-" />
<rule ref="PHPCompatibilityWP" />

<rule ref="WordPress-Core">
<!-- Allow with or without semicolons. -->
<exclude name="Squiz.PHP.EmbeddedPhp.NoSemicolon" />
Expand Down
1 change: 1 addition & 0 deletions composer.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
"wp-coding-standards/wpcs": "1.2.1",
"automattic/vipwpcs": "^0.4.0",
"fig-r/psr2r-sniffer": "^0.5.0",
"phpcompatibility/phpcompatibility-wp": "^1.0.0",
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the mean time v 2.0.0 has come out.... 😕

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for chiming in, Juliette! (We know! 🙈)

I just bumped the package to ^2.0.0, and ensured PHP compatibility checks for 7.1-, as PHP 7.1 is currently the minimum version we both advertise and require in various places such as composer.json files and documentation etc.

Copy link

@jrfnl jrfnl May 1, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tfrommen Thanks!

Just read through the complete thread and there are a few things I can add here.

FYI: This statement from above is not really correct:

Last but not least, I only chose 7.0- as default version range. I mean, I thought it makes sense, because this is what we are using the most, but you can override this in your local config in the repo no problem.

Overruling a PHPCS config setting set within a standard from within a custom ruleset does not work. See this open issue: squizlabs/PHP_CodeSniffer#2197

Since PHPCS 3.3.0 - see squizlabs/PHP_CodeSniffer#1821 - you can overrule a config setting from the command-line though, which in this case you would do with --runtime-set testVersion 5.6- to change the testVersion for a project to 5.6- (= must be compatible with PHP 5.6 and higher).
This kind of command-line addition can, of course, be added to a composer script like:

      "check-cs": [
        "@php ./vendor/squizlabs/php_codesniffer/bin/phpcs --runtime-set testVersion 5.4-"
      ],

We'll be notified things don't work for existing versions while we're developing via Fatals or Warnings. Running a linter for PHP Version compatibility on a regular basis is unnecessary overhead, imo.

This is incorrect. PHPCompatibility will find quite some things for which PHP will not throw any fatals or warning, but where you will see different behaviour between PHP versions. Often these things won't be caught by unit tests either unless a unit test was set up specifically to test for it.

Do these tests inhibit the development feedback cycle by making testing feedback longer?

Barely. If PHP_CodeSniffer is being run anyway, it's only the loading of the rules which will add a second or two, other than that, you will hardly notice it.
FYI: PHPCompatibility is set up to be highly efficient and will bow out as soon as it can for any given sniff. /cc @nathanielks

"squizlabs/php_codesniffer": "~3.4.0",
"dealerdirect/phpcodesniffer-composer-installer": "^0.5.0"
},
Expand Down