Skip to content

Replace usage of substr with str_starts_with and str_ends_with#4395

Closed
Soean wants to merge 11 commits intoWordPress:trunkfrom
Soean:58220
Closed

Replace usage of substr with str_starts_with and str_ends_with#4395
Soean wants to merge 11 commits intoWordPress:trunkfrom
Soean:58220

Conversation

@Soean
Copy link
Member

@Soean Soean commented Apr 28, 2023

Trac ticket: https://core.trac.wordpress.org/ticket/58220


This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.

@spacedmonkey
Copy link
Member

@Soean There seems to be a unit test failure for php 8.1

@Soean
Copy link
Member Author

Soean commented May 25, 2023

@spacedmonkey I merged trunk, now the tests look good.

Copy link
Member

@spacedmonkey spacedmonkey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me!

@spacedmonkey
Copy link
Member

@joelataccelm A backward compatibility shim was added 3cc8f12 ( in WP 5.9.0 ). These are already in core in many many places. WordPress currently supports down to PHP 5.6.

@joelataccelm
Copy link

@joelataccelm A backward compatibility shim was added 3cc8f12 ( in WP 5.9.0 ). These are already in core in many many places. WordPress currently supports down to PHP 5.6.

got it, sorry. I just spotted that in the latest compat file. the version I noticed this fatal erroring on doesn't have the str_ functions in the compat file, so I just need to upgrade.

@sabernhardt
Copy link

According to the Hello Dolly plugin page, it should be compatible with WordPress 4.6 or higher. The str_starts_with function would require either PHP8 or WP5.9.

@SergeyBiryukov
Copy link
Member

Thanks for the PR! Merged in r55990.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants