Skip to content
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

Fix PHP 8 compatibility issues #84

Merged
merged 26 commits into from
Feb 7, 2019
Merged

Fix PHP 8 compatibility issues #84

merged 26 commits into from
Feb 7, 2019

Conversation

nikic
Copy link
Contributor

@nikic nikic commented Feb 7, 2019

This should be a comprehensive fix for all PHP 8 compatibility issues in pear-core. I believe that all the changes done here are both compatible with PHP 5.4 (minimum supported version by PEAR) and not BC breaking.

With this, all tests pass under PHP 8.

cc @ashnazg @torinaki

This is no longer available in PHP 8, and generates an error on
startup.
break and "continue 2" are the same in this case and the code mixes
both. I'm using break here for simplicity.
This flag hasn't been supported since PHP 7, but more importantly
it doesn't even apply to preg_match_all in the first place.
Also adjust one call to it, though this is not strictly necessary.
Also make calls to it static (not strictly necessary) and drop an
object instantiation that becomes unnecessary due to that.
There are many non-static uses of that one, which I'm not touching.
They aren't a problem.
@ashnazg ashnazg self-assigned this Feb 7, 2019
@ashnazg
Copy link
Member

ashnazg commented Feb 7, 2019

@nikic this looks pretty good to me... I think I can do some sanity testing myself in a few hours.

@ashnazg ashnazg merged commit 60ec76f into pear:master Feb 7, 2019
@ashnazg
Copy link
Member

ashnazg commented Feb 7, 2019

@nikic pearweb_phars updated with new release now too.

@nikic
Copy link
Contributor Author

nikic commented Feb 7, 2019

@ashnazg Thanks!

@dbalabka
Copy link
Contributor

dbalabka commented Feb 7, 2019

@nikic, @ashnazg thanks you a lot! I will test PEAR installation.

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.

3 participants