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.
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
Bump WooCommerce minimum required PHP version to 7.4 #39820
Conversation
This includes: - Changing "Requires PHP" in woocommerce.php - Changing "Requires PHP" in readme.txt - Changing "require-php" and "config-platform-php" in composer.json - Changing "testVersion" in phpcs.xml - Updating the composer.lock file with more modern versions of some of the dependencies
Hi @lsinger, Apart from reviewing the code changes, please make sure to review the testing instructions as well. You can follow this guide to find out what good testing instructions should look like: |
Test Results SummaryCommit SHA: 589615f
To view the full API test report, click here. To view the full E2E test report, click here. To view all test reports, visit the WooCommerce Test Reports Dashboard. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for tackling this, @Konamiman!
Looks good, but I do have a few notes and questions:
- let's also update the PHP version in the monorepo root’s
phpcs.xml
. - let's also update 7.3 to 7.4 in
plugins/woocommerce/readme.txt
(down in the prose) - should we also update
phpcs.xml
for woocommerce-beta-tester? - should we also update
plugins/woocommerce-docs/woocommerce-docs.php
? - not sure if
packages/js/create-product-editor-block/plugin-templates/$slug.php.mustache
should also be updated -- there's little point in keeping it at 7.3 I suppose? cc @mattsherman
- "testVersion" in phpcs.xml in the root of the repo - "testVersion" in phpcs.xml in plugins/woocommerce-beta-tester - "Requires PHP" in plugins/woocommerce-docs/woocommerce-docs.php - "Requires PHP" in packages/js/create-product-editor-block/plugin-templates/$slug.php.mustache
bca4606
to
8e7c81f
Compare
…ommerce/create-product-editor-block, woocommerce-beta-tester, woocommerce
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Marking this as "request changes" for now just to remind you that the code freeze for 8.1 is today at 2300 UTC, and that we should hold off on merging this until after the freeze, as we communicated that this wouldn't land until 8.2.
Feel free to dismiss this review once the freeze occurs 😄
We should update that template file to be 7.4 as we are WooCommerce, as it doesn't really make sense to have it be lower than the minimum required version for WooCommerce. |
Thanks for confirming, @mattsherman, I thought as much. It's included in this PR. |
-Change "Requires PHP": - In woocommerce.php - In readme.txt - In plugins/woocommerce-docs/woocommerce-docs.php - In packages/js/create-product-editor-block/plugin-templates/$slug.php.mustache - Change "testVersion" in phpcs.xml: - In the root of the repository - In plugins/woocommerce - In plugins/woocommerce-beta-tester - Change "require-php" and "config-platform-php" in composer.json - Update composer.lock with more modern versions of some of the dependencies - Remove the admin notice about the upcoming bump for PHP 7.3 users in class-wc-admin-notices.php
Changes proposed in this Pull Request:
As announced in the WooCommerce developer blog:
Bump the required PHP version of WooCommerce from 7.3 to 7.4
woocommerce.php
readme.txt
plugins/woocommerce-docs/woocommerce-docs.php
packages/js/create-product-editor-block/plugin-templates/$slug.php.mustache
phpcs.xml
:plugins/woocommerce
plugins/woocommerce-beta-tester
composer.json
composer.lock
with more modern versions of some of the dependenciesclass-wc-admin-notices.php
How to test the changes in this Pull Request:
Test that WooCommerce works and the unit tests still pass. No the best testing instructions ever, but that's pretty much it.
Changelog entry
Significance
Type
Message
Bump required PHP version to 7.4
Comment