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

Compatibility with PHP 8.0 #29

Merged
merged 13 commits into from
Dec 17, 2020
Merged

Compatibility with PHP 8.0 #29

merged 13 commits into from
Dec 17, 2020

Conversation

eclipxe13
Copy link
Contributor

PHPUnit is upgraded since this library supports PHP >= 7.2 and PHPUnit 8 supports from 7.2 until 8.0

  • Fix XSLTProcessor::transformToDoc signature to match with PHP XSLTProcessor.
  • Fix Math functions by casting to floats.
  • Update tests to work with PHPUnit 8.
  • Travis-CI: Fixed compatibility with XDebug 3.0.
  • Travis-CI: Due compatibility issues with PHP 8.0, do not install or test php-cs-fixer and phpstan.

Important: One pending task (that requires a different PR) is to upgrade PHPStan from version 0.11 to version 0.12.

@frederikbosch
Copy link
Contributor

Nice! We could drop 7.2 and only support 7.3/7.4/8.0 now. What do you think?

@eclipxe13
Copy link
Contributor Author

It's up to you, PHP 7.2 has reached EOL since 2020-11-30.
If PHP 7.2 is drop then we can upgrade PHPUnit to version 9.

@frederikbosch
Copy link
Contributor

Let's go for PHP7.3+ only. I rather not support version that reached EOL.

@eclipxe13
Copy link
Contributor Author

Just try to make it work upgrading to PHPUnit 9, there are two files that require to change on the test suite:

  • assertRegExp is deprecated, substitute with assertMatchesRegularExpression
  • expectException(Error::class) should be replaced with expectError(), also expectExceptionMessage() replace to expectErrorMessage()

The main issue I see on upgrade to PHPUnit 9 is that it has incompatible dependencies with PHPStan 0.11.
Also PHPStan 0.11 has been deprecated since 2019-10-22,

So, I would recommend to you follow the path:

  1. Merge this PR that allows this library to run using PHP 8.0
  2. Upgrade to PHPStan 0.12 and solve the all the issues (this is not trivial, level 6 is tedious)
  3. Upgrade to PHPUnit 9 and PHP >= 7.3

PHPStan 0.12 counts 175 errors on max level:

  • Level 2: 3 trivial errors
  • Level 4: +8 errors, trivial but requires your review
  • Level 6: +91 errors, need a lot of annotations on traversables and arrays
  • Level 7: +8 errors
  • Level 8: +65 errors

I can work with you on this process, what is your opinion?

@frederikbosch
Copy link
Contributor

Let's include the drop of PHP 7.2 in this PR by just removing it from the testsuite. Then I'll merge. I agree with you on the rest of path.

@frederikbosch
Copy link
Contributor

I can do the PHPStan upgrade to 0.12 no problem. I have done it before for my mail package. Mostly it is improving docblocks with generics syntax.

For each version the build runs only 2 times, not 3 as before.
Remove PHP 8.0 from allow failures section
@frederikbosch frederikbosch merged commit 71067a2 into genkgo:master Dec 17, 2020
@frederikbosch
Copy link
Contributor

Fantastic, great work!

@eclipxe13 eclipxe13 deleted the php8.0 branch December 17, 2020 19:03
@vkondratjevs
Copy link

You can crate tag to php8 ?

@frederikbosch
Copy link
Contributor

Yes, done!

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