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

Allow php 8 #6

Merged
merged 5 commits into from
Jan 18, 2021
Merged

Allow php 8 #6

merged 5 commits into from
Jan 18, 2021

Conversation

holtkamp
Copy link
Contributor

Allow installing this provider on PHP 8

Copy link
Member

@JaZo JaZo left a comment

Choose a reason for hiding this comment

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

Nice work @holtkamp! However, I'd like to keep PHP 7.0 as minimum required version for now. Also don't forget to update CI config (.travis.yml and .scrutinizer.yml) to run tests on all supported PHP versions.

composer.json Outdated Show resolved Hide resolved
@JaZo JaZo self-assigned this Jan 14, 2021
@holtkamp
Copy link
Contributor Author

holtkamp commented Jan 15, 2021

I am afraid 7.2 will be the minimum since PHPUnit requires / allows this and higher versions (required for PHP 8) since 8.5.12, I could give that a try...?

But 7.4 gives all the niceness of typing, etc, so that would be my suggestion. A lot of libraries use 7.4 as minimum now.

The Geocoder itself aims at 7.3:

https://github.com/geocoder-php/Geocoder/blob/3249d994244d30f591a71b1911ca936bd35f71b4/composer.json#L20

Also note that security fixes have not been supported anymore for 7.0 since january 2019

@JaZo
Copy link
Member

JaZo commented Jan 18, 2021

I see supporting PHP 7.0 will need some workarounds and I agree it's not worth it. I'm aware of all the niceness in PHP 7.4, but for this one class library I don't feel the urge to have those available so then I'd like to go for PHP 7.2 as minimum.

@holtkamp
Copy link
Contributor Author

holtkamp commented Jan 18, 2021

Tried it with "php": "^7.2 || ^8.0" and "phpunit/phpunit": "^8.5" but then geocoder-php/provider-integration-tests is unable to install since none of the tagged versions allows php unit 8.5:

Problem 1
    - geocoder-php/provider-integration-tests 1.5.0 requires phpunit/phpunit ^9.5 -> satisfiable by phpunit/phpunit[9.5.0, 9.5.1, 9.5.x-dev] but these conflict with your requirements or minimum-stability.
    - geocoder-php/provider-integration-tests 1.4.1 requires phpunit/phpunit ^7.5 -> satisfiable by phpunit/phpunit[7.5.0, 7.5.1, 7.5.10, 7.5.11, 7.5.12, 7.5.13, 7.5.14, 7.5.15, 7.5.16, 7.5.17, 7.5.18, 7.5.19, 7.5.2, 7.5.20, 7.5.3, 7.5.4, 7.5.5, 7.5.6, 7.5.7, 7.5.8, 7.5.9] but these conflict with your requirements or minimum-stability.

The combination "php": "^7.3 || ^8.0" and "phpunit/phpunit": "^9.0" works and installs PHPUnit 9.5, so I guess the minimum version of PHP is forced to be 7.3...?

@JaZo
Copy link
Member

JaZo commented Jan 18, 2021

The testsuite is compatible with PHPUnit 7-9, so we can use "phpunit/phpunit": "^7.0|^8.0|^9.0" to allow running tests on PHP 7.2.

@JaZo JaZo merged commit dc227ac into swisnl:master Jan 18, 2021
@JaZo
Copy link
Member

JaZo commented Jan 18, 2021

Thanks for your effort @holtkamp, released in 1.3.0!

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.

None yet

2 participants