search

Automattic / VIP-Coding-Standards

PHP_CodeSniffer ruleset to enforce WordPress VIP coding standards.
https://wpvip.com/documentation/how-to-install-php-code-sniffer-for-wordpress-com-vip/
Other
236 stars 40 forks source link

Raise minimum version of PHP? #211

Closed GaryJones closed 6 years ago

GaryJones commented 6 years ago

In dropping support for PHPCS 2.x, the minimum version of PHP was raised to PHP >=5.4.

@tomjn has proposed raising this requirement for running VIPCS to at least "a supported version PHP version like 5.6".

Thoughts?

tomjn commented 6 years ago

imo PHPCS should run on PHP 7.2, even if the code targets unsupported versions of PHP. PHPCompat and PHPUnit can test the plugins and themes in 5.2/5.4 if it's necessary, but that doesn't require PHPCS to run on those versions too

If necessary, they can add the PHPCompatibility ruleset as a dependency to add another layer of protection in addition to VIPCS

GaryJones commented 6 years ago

For VIPCS Travis, we run PHPCS against the VIPCS code only at 7.2:

For VIPCS unit tests in Travis, it needs a composer install, though there may be a way for it to use any of the desired versions PHP, while ignoring any PHP requirement set for VIPCS itself. Maybe platform (which WP-CLI uses)?

rinatkhaziev commented 6 years ago

PHP5.6 will be officially EOL'd soon (Dec 3, 2018): http://php.net/supported-versions.php

Personally, I think the requirement should be at least 7.0, preferably 7.2 since it's what we run.

GaryJones commented 6 years ago

Personally, I think the requirement should be at least 7.0, preferably 7.2 since it's what we run.

There is definitely some logic there. Any developers wanting to check code that is to be run on PHP 7.2, should likely have a version of PHP 7.2 available with which to run PHPCS, even if that's done inside their local VM.

pyronaur commented 6 years ago

While I agree that every web server and dev should have PHP7+ on their machine, I think that we should require a version not for the name of the version, but for the features we rely upon.

Unless we decide that we need anonymous classes or scalar type hints to improve the tooling, I see no good reason to make PHP7 a requirement.

If we base this decision on PHP Supported versions then we need to require at least PHP 7.1 instead of PHP 7 because security updates for PHP7 stop sooner than for PHP 5.6

GaryJones commented 6 years ago

https://github.com/Automattic/VIP-Coding-Standards/commit/e49101423acbd74f8f9aa292ee1c6b82875d8ef5 changed the required PHP to >=5.6.

Unless we decide that we need anonymous classes or scalar type hints to improve the tooling, I see no good reason to make PHP7 a requirement.

PHPCS, and WPCS PHPCompatibility and other standards don't require PHP 7, so I think we're probably good for now.