Bump minimum php to 7.2 for next major #301
Conversation
michalbundyra
left a comment
michalbundyra
left a comment
There was a problem hiding this comment.
Is it just the bump (and PHPUnit upgrade) and other changes will be in separate PR? I would expect to use some PHP 7.2 features in the next major release.
| @@ -1,7 +1,10 @@ | |||
| .phpunit.result.cache | |||
There was a problem hiding this comment.
Why there is no slash at the beginning? Can it be in any place?
There was a problem hiding this comment.
Yes.
It was placed in folder next to a test when I run just that specific test from PhpStorm. Probably CWD was not a project root.
| "forum": "https://discourse.zendframework.com/c/questions/components" | ||
| }, | ||
| "require": { | ||
| "php": "^7.2", |
| */ | ||
| public function testInvalidOptionType() | ||
| { | ||
| $this->expectException(ServiceNotCreatedException::class); |
There was a problem hiding this comment.
For me it will be much clearer to put these expectations just before the function which should throw the exception.
In that case it doesn't make a big difference, as the other line here is mocking, but in many case it could be wrong adding it at the very beginning part of the test.
There was a problem hiding this comment.
This exactly mirrors deprecated annotation behavior.
Whole test suite needs a few improvement passes. Not in this PR/at this time.
It is just a baseline PR. Next will be coding standard upgrade, followed by typehints PR followed by actual major changes |
3 participants
Note: PHP 7.1 is out of active support and have only 9 months of security support left at this time.