GH Actions: tweak the way the PHPCS/WPCS versions are set #787
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
VIPCS now doesn't just have PHPCS and WPCS as dependencies for the sniffs and ruleset tests, but also PHPCSUtils, PHPCSExtra and VariableAnalysis and for those last three, the different versions which may be supported are not (yet) taken into account. Now, it could be argued that every single combination of the different versions of each of these dependencies should be tested, but that would make the matrix _huge_ to little added benefit. So, instead I'm proposing a slightly different strategy, which should still allow us to verify that things work correctly with enough confidence, while making the workflow maintenance less involved. The change I'm proposed in this commit takes advantage of the Composer `--prefer-lowest` option to achieve this. It basically sets the matrix up to test against a combination of all CS dependencies on their lowest supported version + on their stable/highest supported version. While at this time, the lowest and the stable versions are the same, this will not always be the case, so having the matrix set up this way allows for new releases of these CS dependencies automatically. In the original setup, the highest/stable version combi wasn't tested. Instead a combi using the `dev` version of the dependencies was used. To me, it makes sense to test against the `dev` versions as well, but I don't believe this needs to be done for the whole range of supported PHP versions. To that end, I've set up four extra jobs against select high/low PHP versions in the `test` workflow to test against a combination of all CS dependencies on their latest `dev` version. Note: I have not added the setup for testing against `dev` versions to the `quicktest` workflow. Also note that the workflows currently contain a toggle for installing the `lowest` versions with/without ignoring platform requirement. This toggle is needed for PHP 8.x due to the max supported PHPUnit version being PHPUnit 7.x. This toggle can be removed once upstream PR squizlabs/PHP_CodeSniffer 3803 has been merged.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
VIPCS now doesn't just have PHPCS and WPCS as dependencies for the sniffs and ruleset tests, but also PHPCSUtils, PHPCSExtra and VariableAnalysis and for those last three, the different versions which may be supported are not (yet) taken into account.
Now, it could be argued that every single combination of the different versions of each of these dependencies should be tested, but that would make the matrix huge to little added benefit.
So, instead I'm proposing a slightly different strategy, which should still allow us to verify that things work correctly with enough confidence, while making the workflow maintenance less involved.
The change I'm proposed in this commit takes advantage of the Composer
--prefer-lowestoption to achieve this.It basically sets the matrix up to test against a combination of all CS dependencies on their lowest supported version + on their stable/highest supported version.
While at this time, the lowest and the stable versions are the same, this will not always be the case, so having the matrix set up this way allows for new releases of these CS dependencies automatically.
In the original setup, the highest/stable version combi wasn't tested. Instead a combi using the
devversion of the dependencies was used. To me, it makes sense to test against thedevversions as well, but I don't believe this needs to be done for the whole range of supported PHP versions. To that end, I've set up four extra jobs against select high/low PHP versions in thetestworkflow to test against a combination of all CS dependencies on their latestdevversion.Note: I have not added the setup for testing against
devversions to thequicktestworkflow.Also note that the workflows currently contain a toggle for installing the
lowestversions with/without ignoring platform requirement. This toggle is needed for PHP 8.x due to the max supported PHPUnit version being PHPUnit 7.x. This toggle can be removed once upstream PR squizlabs/PHP_CodeSniffer#3803 has been merged.