Make WordPress Core

Opened 7 years ago

Closed 7 years ago

#42789 closed task (blessed) (fixed)

Bump recommended PHP version from 7.0 -> 7.2

Reported by: rachelbaker's profile rachelbaker Owned by: rachelbaker's profile 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 7 years ago.

Download all attachments as: .zip

Change History (22)

#1 @rachelbaker
7 years 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
7 years ago

#2 @rachelbaker
7 years 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.


7 years ago

#4 follow-up: @johnbillion
7 years 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
7 years ago

  • Keywords has-patch needs-refresh added

#6 in reply to: ↑ 4 @rachelbaker
7 years 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
7 years ago

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

#8 @jorbin
7 years 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
7 years ago

@otto42 made the changes on .org to say 7.2

#10 @johnbillion
7 years ago

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

#11 @johnbillion
7 years ago

  • Milestone changed from Awaiting Review to 5.0

#12 @netweb
7 years 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
7 years 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
7 years ago

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

https://core.trac.wordpress.org/ticket/42860

Version 0, edited 7 years ago by lisota (next)

#15 follow-up: @SergeyBiryukov
7 years 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
7 years 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: @kirasong
7 years 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 7 years ago by kirasong (previous) (diff)

#18 in reply to: ↑ 17 ; follow-up: @dd32
7 years 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
7 years 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 7 years ago by SergeyBiryukov (previous) (diff)

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


7 years ago

#21 @ocean90
7 years 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.