Also tests for PHP 7.1. #69
Conversation
1 similar comment
2 similar comments
There was a problem hiding this comment.
Great to have more contributions @lucasmichot. PHP 7.1 I definitely something that needs to be added to the test matrix.
| @@ -13,7 +13,8 @@ | |||
| "require-dev": { | |||
| "php-vcr/phpunit-testlistener-vcr": "^1.1", | |||
| "jakub-onderka/php-parallel-lint": "^0.9.2", | |||
| "jakub-onderka/php-console-highlighter": "^0.3.2" | |||
| "jakub-onderka/php-console-highlighter": "^0.3.2", | |||
| "phpunit/phpunit": "~4.8 || ~5.0" | |||
There was a problem hiding this comment.
Not a fan of this, I've had conflicts with dependencies I've considered integrating with the SDK before. Whatever can safely stay outside of composer should. (I realise that's a bit of a pain in a number of ways but pain > dependency conflicts)
Instead I'd modify this line to conditionally load a different version of phpunit. https://github.com/contentful/contentful.php/pull/69/files#diff-354f30a63fb0907d4ad57269548329e3L21
There was a problem hiding this comment.
The fact is that this dependency has actually been properly resolved by composer. see: https://travis-ci.org/contentful/contentful.php/builds/215916416
Also this allows any people to test locally PR they submit to this repo, without forcing them to install globally a specific phpunit version
There was a problem hiding this comment.
The same issue with phar apply here: https://github.com/contentful/contentful.php/pull/70/files#r108655179
| - composer self-update | ||
| - composer install | ||
| - travis_retry composer self-update | ||
| - travis_retry composer install |
There was a problem hiding this comment.
Interesting, did you have problems with composer failing?
There was a problem hiding this comment.
It's a common practice to avoid timeouts: https://docs.travis-ci.com/user/common-build-problems/
|
I'll see what it takes to drop PHP 5.5, not sure if that's gonna work out though. |
Also tests for PHP 7.1.