Update phpcompatibility to develop version to get sniffs for PHP 8 #9697
Conversation
… as the latest stable one only covers up to PHP 7.4.
alopezari
added
the
focus: devops
Test the buildOption 1. Jetpack Beta
Option 2. Jurassic Ninja - available for logged-in A12s🚀 Launch a JN site with this branch 🚀 ℹ️ Install this Tampermonkey script to get more options. Build info:
Note: the build is updated when a new commit is pushed to this PR. |
|
Size Change: 0 B Total Size: 1.34 MB ℹ️ View Unchanged
|
There was a problem hiding this comment.
@alopezari The current changes in this PR looks good.
However, looking at the PHP compatibility workflow, we're using a different config file for compatibility tests - https://github.com/Automattic/woocommerce-payments/blob/develop/phpcs-compat.xml.dist
You'll need to update the above config to test PHP versions 7.3 to 8.4.
Also, it would be great to have an npm script for running compatibility tests locally. Can you add that as well?
There was a problem hiding this comment.
@alopezari Looking at the compatibility test result of this PR, I noticed this error:
The function str_starts_with() is not present in PHP version 7.4 or earlier
I don't think it's an actual issue because our code works as expected on PHP 7.3 and above. If this was an actual issue, we would've received fatal errors. May be we're using some polyfills. Can you look into this as well?
|
Thanks for the review @achyuthajoy!
Yes! I'm aware of that. I left it as it is because it is
Sure thing!
Interesting, will check it! |
…path from phpcs-compat.xml.dist.
Added in e6f4ccb!
Okay, this error was found only on jetpack code. Our current It makes sense to me to focus our tests and checks on the code we build, not dependencies, so - in addition to |
* develop: Wait for Stripe js to load before using it (#9770) Remove redundant stripe payment elements mount for pay for order (#9813) Enforce proper return types for methods get_order_from_event_body (#9761) Update phpcompatibility to develop version to get sniffs for PHP 8 (#9697) Enable ECE Tracks Events when WooPay is disabled (#9793) chore: rename PRB constants to ECE (#9768) Amend changelog entries for release 8.5.1 fix: remove 'test mode' badge from shortcode checkout (#9800) Update version and add changelog entries for release 8.5.1
Closes 370-gh-Automattic/woocommerce-payments-meta
Changes proposed in this Pull Request
We use a PHPCS extension - PHPCompatibility - to check the compatibility of our plugin with multiple PHP versions.
We were using the latest stable version 9.3.5. However, this version was released in 2019 and it doesn't cover rules for PHP 8.
This PR adds the following changes:
dev-developversion of PHPCompatibility to include sniffs for all PHP 8 versions, including new sniffs for PHP 8.4 - yet to be released.dev-developversion was tagged as fake9.3.5to keep compatibility with other dependencies.phpcs.xml.distconfig file are 7.3-7.4. This is because our code is fully compatible with these versions but there are some warnings and errors for PHP 8, which would break our pipelines sincephpcs.xml.distrules are required for PRs to be merged.This way, we will be able to spot potential incompatibilities with PHP 8.4 before this PHP version is out.
Note
A follow up task will be created to fix the PHP 8 warnings and errors.
Testing instructions
npm run lint:php -- --no-cacheand check that no errors appear../vendor/bin/phpcs --standard=phpcs-compat.xml.dist $(git ls-files | grep .php$)and check that multiple errors and warnings appear, most of all of them related to PHP 8 incompatibilities (you can see in the log the PHP version each error/warning is related to).PR Compatibility, that should fail due to PHP 8 errors and warnings.npm run changelogto add a changelog file, choosepatchto leave it empty if the change is not significant. You can add multiple changelog files in one PR by running this command a few times.Post merge