[PhpUnitBridge] install PHPUnit 7 on PHP 7.1 and fix requir. for PHPUnit 6 #29439
Conversation
|
@derrabus thanks for review 👍 I changed comments. |
|
That should be submitted on master: we shouldn't change the version on a patch release. |
84fafe8 to
79d771b
Compare
|
@nicolas-grekas thank you for remark. Changed. |
fb107eb to
e5c7c6b
Compare
There was a problem hiding this comment.
Also I found that PHPUnit 6.5 required PHP 7.0, not 7.2
Comment you deleted does not say that you need to use PHP 7.2 for PHPUnit 6. It explains that PHP 7.2 can't be used with PHPUnit 5. You are free to use PHPUnit 5 until version 7.2.
Comments you reversed no longer explain why is given phpunit version installed for given php version. And suddenly installing phpunit 6 for php 7.0-7.1 and phpunit 7 for php>=7.2 by default will probably break the builds for lot of people.
This PR significantly changes current strategy from installing lowest working PHPUnit version for given PHP version to installing latest PHPUnit version.
14f1b75 to
a63cb9e
Compare
User is free to setup
The comment was wrong. PHPUnit 5 can be used with PHP 7.2 and also 7.3... so, that's why I changes the comment. |
I know, doesn't change the fact that it will by default update major phpunit version without asking.
In that case condition should have been removed to install PHPUnit 5, which would follow current strategy and there is no need for PHPUnit 6 and 7 at all. At the time there were issues with it #27370 but it appears it has been resolved since then. For the record, I am for using newer versions, but it needs to be highlighted that this PR is not about fixing the requirement, but about installing latest PHPUnit version possible for given PHP version, which changes the strategy that was used until now that was using oldest major working for given PHP version. |
do you have any suggestions how to highlight that? Generally it's the single scope of this PR. |
|
No need, I did highlight it with my comment. It needs comment from symfony members now status: needs review |
|
Thank you @gregurco. |
…equir. for PHPUnit 6 (gregurco) This PR was merged into the 4.3-dev branch. Discussion ---------- [PhpUnitBridge] install PHPUnit 7 on PHP 7.1 and fix requir. for PHPUnit 6 | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | | License | MIT | Doc PR | Added support of PHPUnit 7.4 if PHP version is 7.1+ (release link: https://packagist.org/packages/phpunit/phpunit#7.4.5). Also I found that PHPUnit 6.5 required PHP 7.0, not 7.2 (proof: https://packagist.org/packages/phpunit/phpunit#6.5.13) Commits ------- 30609bf [PhpUnitBridge] install PHPUnit 7 on PHP 7.1 and fix require for PHPUnit 6
Added support of PHPUnit 7.4 if PHP version is 7.1+ (release link: https://packagist.org/packages/phpunit/phpunit#7.4.5).
Also I found that PHPUnit 6.5 required PHP 7.0, not 7.2 (proof: https://packagist.org/packages/phpunit/phpunit#6.5.13)