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

PHP 7.3 #15

Merged
merged 5 commits into from
Oct 13, 2019
Merged

PHP 7.3 #15

merged 5 commits into from
Oct 13, 2019

Conversation

brandonramirez
Copy link
Contributor

@brandonramirez brandonramirez commented Oct 13, 2019

Note that PHP 7.2 and 7.3 show tons of warnings about null not being Countable, however, they do not break the build.

It's a known issue in this version of PHPUnit. There is a fix, but the maintainer of PHPUnit is not interested in back-porting to PHPUnit 4.x or even 5.x.
sebastianbergmann/php-code-coverage#554

PHPUnit 4.x PR: sebastianbergmann/php-code-coverage#590
PHPUnit 5.x PR: sebastianbergmann/php-code-coverage#591

So in order for tests to run perfectly clean under PHP 7.2 and 7.3, we need to upgrade PHPUnit to at least 6.x, which kills support for PHP 5.x (minimum supported: PHP 7.0)

I think we can live with the warnings for the time being, but should consider dropping support for older PHP versions in the near future.

@brandonramirez brandonramirez marked this pull request as ready for review October 13, 2019 12:40
@mpetrovich
Copy link
Owner

mpetrovich commented Oct 13, 2019

I agree about dropping pre-7.x support. For simplicity and maintainability, we should start following the official PHP version support schedule: https://www.php.net/supported-versions.php

Interestingly, this would mean dropping support for 7.0 immediately and for 7.1 in January.

@mpetrovich mpetrovich merged commit a9dae1b into mpetrovich:master Oct 13, 2019
@brandonramirez brandonramirez deleted the php-7.3 branch October 13, 2019 21:54
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.

2 participants