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 version upgrade to 7.4 and PHPUnit upgrade to ^9.0. #29

Merged
merged 3 commits into from
Feb 5, 2022

Conversation

ales-k
Copy link
Contributor

@ales-k ales-k commented Feb 2, 2022

PHP 8 disables libxml_disable_entity_loader by default:
https://php.watch/versions/8.0/libxml_disable_entity_loader-deprecation

PHPUnit - changing method expectExceptionMessageRegExp() to expectExceptionMessageMatches():
sebastianbergmann/phpunit#4133


I actually tested executing in PHP 8.0 and it worked. I also executed phpunit tests which (after my changes) passed. However I am new to contributing so I will be really glad for any feedback or advice how to make sure that my changes are correct and actually helpful.

@mougrim
Copy link
Owner

mougrim commented Feb 2, 2022

Hi!
Thanks for contribution!

As I see php7.4 is supported yet.
So, please, provide minimum php version 7.4 in composer.json.

For libxml_disable_entity_loader you can use suggest form your link:

- libxml_disable_entity_loader(true);
+ if (\PHP_VERSION_ID < 80000) {
+      libxml_disable_entity_loader(true);
+ }

Please, fix it.

Also, could you fix .travis.yml? Need to remove old php version and add 7.4, 8.0, 8.1 (if supported by travis).

@mougrim
Copy link
Owner

mougrim commented Feb 2, 2022

Sorry, looks like travis doesn't work now, so, isn't need to change .travis.yml.

@ales-k
Copy link
Contributor Author

ales-k commented Feb 3, 2022

Thanks for your fast reply! Changes done.

@mougrim
Copy link
Owner

mougrim commented Feb 3, 2022

Thanks!

php8.1 and php8.0 are ok.
On php7.4 got errors in tests:

ParseError: syntax error, unexpected '|', expecting variable (T_VARIABLE)

/vendor/psr/log/src/LoggerInterface.php:30
/vendor/monolog/monolog/src/Monolog/Logger.php:34
/tests/Unit/Xml/DomXmlConverterTest.php:405
/tests/Unit/Xml/DomXmlConverterTest.php:28
phpvfscomposer:///vendor/phpunit/phpunit/phpunit:97

Looks like need to update composer.lock.

Also, could you update CHANGELOG.md?

@mougrim mougrim changed the title PHP version upgrade to 8.0 and PHPUnit upgrade to ^9.0. PHP version upgrade to 7.4 and PHPUnit upgrade to ^9.0. Feb 3, 2022
@mougrim
Copy link
Owner

mougrim commented Feb 5, 2022

Thanks! Will add composer.lock later with code-style fixes (after update cs tool).

@mougrim mougrim merged commit f49dd01 into mougrim:master Feb 5, 2022
This was referenced Feb 5, 2022
mougrim added a commit that referenced this pull request Feb 5, 2022
There are next changes:

- Minimum PHP version supported upgraded to 7.4 (#29)
- PHPUnit version upgraded to 9 (#29)
- Add run unit tests Github action (#30, #31)
- Remove travis (#30)
- Fix code style issues (#32)
- Fix by workaround grumphp issue with TypeError, see phpro/grumphp#957 (#32)
- Update friendsofphp/php-cs-fixer to 3 (#33)
- Add psalm (#34, #35)
- Add check cs github action (#36)
mougrim added a commit that referenced this pull request Feb 5, 2022
There are next changes:

- Minimum PHP version supported upgraded to 7.4 (#29)
- PHPUnit version upgraded to 9 (#29)
- Add run unit tests Github action (#30, #31)
- Remove travis (#30)
- Fix code style issues (#32)
- Fix by workaround grumphp issue with TypeError, see phpro/grumphp#957 (#32)
- Update friendsofphp/php-cs-fixer to 3 (#33)
- Add psalm (#34, #35)
- Add check cs github action (#36)
@mougrim
Copy link
Owner

mougrim commented Feb 5, 2022

@ales-k, released v0.5.0

@ales-k
Copy link
Contributor Author

ales-k commented Feb 11, 2022

Thank you very much for your help.

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