WordPress.org

Make WordPress Core

#42789 closed task (blessed) (fixed)

Bump recommended PHP version from 7.0 -> 7.2

Reported by: rachelbaker Owned by: rachelbaker
Milestone: 4.9.5 Priority: normal
Severity: normal Version:
Component: General Keywords: has-patch fixed-major
Focuses: Cc:

Description

The recommended version of PHP in the readme.html file is 7.0. As of December 3, 2017, PHP 7.0 is no longer actively supported. The recommended version should be bumped to 7.1

Attachments (1)

42789.diff (698 bytes) - added by rachelbaker 21 months ago.

Download all attachments as: .zip

Change History (22)

#1 @rachelbaker
21 months ago

Failing unit test:

1) Tests_External_HTTP_Basic::test_readme
readme.html's Recommended PHP version is too old. Remember to update the WordPress.org Requirements page, too.
Failed asserting that an array contains '7'.
/home/travis/build/WordPress/wordpress-develop/tests/phpunit/tests/external-http/basic.php:23

@rachelbaker
21 months ago

#2 @rachelbaker
21 months ago

  • Owner set to rachelbaker
  • Status changed from new to accepted

In 42789.diff bumping the recommended PHP version to 7.1, which is under active support until December 1, 2018.

This ticket was mentioned in Slack in #core by rachelbaker. View the logs.


21 months ago

#4 follow-up: @johnbillion
21 months ago

  • Type changed from defect (bug) to task (blessed)

PHP 7.2 was released last week. WordPress 4.9 is fully tested and compatible. Should that be the recommended version?

#5 @rachelbaker
21 months ago

  • Keywords has-patch needs-refresh added

#6 in reply to: ↑ 4 @rachelbaker
21 months ago

Replying to johnbillion:

PHP 7.2 was released last week. WordPress 4.9 is fully tested and compatible. Should that be the recommended version?

@johnbillion Aah. I didn’t realize we were able to recommend 7.2 yet. Honestly, I don’t even know if there is a process around what versions we recommend. I will leave this open for others to weigh in as well.

#7 @johnbillion
21 months ago

I think we can just yolo it to 7.2. There's no reason _not_ to recommend 7.2.

#8 @jorbin
21 months ago

I've been running on 7.2 and it is a smooth experience. Let's go for 7.2.

@pento - Can you commit that and also update https://wordpress.org/about/requirements/ ?

#9 @jorbin
21 months ago

@otto42 made the changes on .org to say 7.2

#10 @johnbillion
21 months ago

  • Resolution set to fixed
  • Status changed from accepted to closed

#11 @johnbillion
21 months ago

  • Milestone changed from Awaiting Review to 5.0

#12 @netweb
21 months ago

  • Summary changed from Bump recommended PHP version from 7.0 -> 7.1 to Bump recommended PHP version from 7.0 -> 7.2

#13 @ayeshrajans
21 months ago

Awesome to hear about bumping version to 7.2! I work quite a lot on legacy code, and from what I have seen, PHP 7.2 can raise some PHP notices that are often not seen from tests or and pop up in edge cases. I have worked on a few tickets for 7.2 and tests are passing(yay!), but personally, I think there is still a room for improvement.

One very frequent problem with 7.2 is that count() calls on uncountable variables raise a notice. This kind of bugs are very difficult to grep from the repo and often missed from tests. count(), each() and such deprecation notices are easy to fix but annoying nonetheless. If we can actively hunt those bugs quickly as they are reported, that can help a lot.

#14 @lisota
21 months ago

Here is an example count() bug, referenced by @ayeshrajans

#42860

Last edited 18 months ago by SergeyBiryukov (previous) (diff)

#15 follow-up: @SergeyBiryukov
18 months ago

  • Keywords fixed-major added; needs-refresh removed
  • Milestone changed from 5.0 to 4.9.5
  • Resolution fixed deleted
  • Status changed from closed to reopened

Reopening for 4.9.5 consideration.

@mikeschroder noted that the test still fails on the 4.9 branch. Per comment:4, it should be safe to bump the recommended version there as well.

There were some additional fixes: #35560, #43201, #43216; some more are coming: #42814, #43312.

#16 in reply to: ↑ 15 ; follow-up: @dd32
18 months ago

Replying to SergeyBiryukov:

Reopening for 4.9.5 consideration.

@mikeschroder noted that the test still fails on the 4.9 branch. Per comment:4, it should be safe to bump the recommended version there as well.

While it should be safe, the test is only supposed to run on master/trunk: (Although that check is specifically only for Travis AFAICT)
https://core.trac.wordpress.org/browser/trunk/tests/phpunit/tests/external-http/basic.php?marks=8,9

#17 in reply to: ↑ 16 ; follow-up: @mikeschroder
18 months ago

Replying to dd32:

Replying to SergeyBiryukov:

Reopening for 4.9.5 consideration.

@mikeschroder noted that the test still fails on the 4.9 branch. Per comment:4, it should be safe to bump the recommended version there as well.

While it should be safe, the test is only supposed to run on master/trunk: (Although that check is specifically only for Travis AFAICT)
https://core.trac.wordpress.org/browser/trunk/tests/phpunit/tests/external-http/basic.php?marks=8,9

I noticed when running the tests locally on the 4.9 branch before a backport.

Edit: In case it's not supposed to work like this, running grunt test or grunt precommit locally both appear to run this test when the 4.9 branch is checked out.

Last edited 18 months ago by mikeschroder (previous) (diff)

#18 in reply to: ↑ 17 ; follow-up: @dd32
18 months ago

Replying to mikeschroder:

Replying to dd32:
I noticed when running the tests locally on the 4.9 branch before a backport.

Edit: In case it's not supposed to work like this, running grunt test or grunt precommit locally both appear to run this test when the 4.9 branch is checked out.

Yeah, That's kind of what I'm hinting at - We can't backport these changes forever to branches we're backporting to. Backporting it to our current stable seems like a sane idea, but once 5.0 is out, the unit test would once again start failing on 4.9 backports.
For 4.9, I say back-porting it is OK, but that we should look at something else going forward.

#19 in reply to: ↑ 18 @SergeyBiryukov
18 months ago

Replying to dd32:

For 4.9, I say back-porting it is OK, but that we should look at something else going forward.

Maybe adjust the test to skip when running on a 3-digit version?

Last edited 18 months ago by SergeyBiryukov (previous) (diff)

This ticket was mentioned in Slack in #core by audrasjb. View the logs.


17 months ago

#21 @ocean90
17 months ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 42847:

Readme: Update recommended PHP version to 7.2.

Merge of [42358] to the 4.9 branch.

Props otto42, johnbillion, rachelbaker, jorbin.
Fixes #42789.

Note: See TracTickets for help on using tickets.