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

GH Actions: test against different versions of all CS dependencies #2417

Merged
merged 1 commit into from
Dec 14, 2023

Conversation

jrfnl
Copy link
Member

@jrfnl jrfnl commented Dec 13, 2023

As things were, when testing against "dev" versions, only PHPCS "dev" was used, while WPCS now has more dependencies, i.e. PHPCSUtils and PHPCSExtra.

This commit updates the workflows to take that into account better.
It also improves the readability of the workflows by using multi-line command in a few places.

Notes:

  • This renames the phpcs_version matrix variable to dependencies in all workflows to make it clear this is not just about PHPCS itself, but about all dependencies.
  • The CS check for the own codebase is always run against "dev" versions of all dependencies as an early detection system for upstream issues.
  • The "Ruleset" checks are run against low/stable/dev.
  • The fixer conflict check is run against dev only, as conflicts detected cannot be fixed anymore in already released versions.
  • The "quick check" now only runs against low/stable.
  • The "test" run run against both low/stable by default and now has some extra builds against "dev" versions.
    • These extra builds could be set to "continue on error", but I've chosen not to do so as if any issues are detected, they should be fixed asap as they will impact WPCS sooner rather than later.
    • Code coverage will now be run against low/high PHP with low/stable dependencies and no longer against dev.
  • Also note that, while the "quick test"/"test" runs shouldn't need PHPCSExtra, as that is a dependency used only in the ruleset, not in our sniff code, I've still included PHPCSExtra in the upgrade/downgrade block for consistency.

These changes do mean that the "required build"/branch protection will need to be updated (again) before the PR can be merged.
I will do so once the PR has been approved.

As things were, when testing against "dev" versions, only PHPCS "dev" was used, while WPCS now has more dependencies, i.e. PHPCSUtils and PHPCSExtra.

This commit updates the workflows to take that into account better.
It also improves the readability of the workflows by using multi-line command in a few places.

Notes:
* This renames the `phpcs_version` matrix variable to `dependencies` in all workflows to make it clear this is not just about PHPCS itself, but about all dependencies.
* The CS check for the own codebase is always run against "dev" versions of all dependencies as an early detection system for upstream issues.
* The "Ruleset" checks are run against low/stable/dev.
* The fixer conflict check is run against dev only, as conflicts detected cannot be fixed anymore in already released versions.
* The "quick check" now only runs against low/stable.
* The "test" run run against both low/stable by default and now has some extra builds against "dev" versions.
    - These extra builds could be set to "continue on error", but I've chosen not to do so as if any issues are detected, they should be fixed asap as they will impact WPCS sooner rather than later.
    - Code coverage will now be run against low/high PHP with low/stable dependencies and no longer against dev.
* Also note that, while the "quick test"/"test" runs shouldn't need PHPCSExtra, as that is a dependency used only in the ruleset, not in our sniff code, I've still included PHPCSExtra in the upgrade/downgrade block for consistency.

These changes do mean that the "required build"/branch protection will need to be updated (again) before the PR can be merged.
I will do so once the PR has been approved.
Copy link
Member

@dingo-d dingo-d left a comment

Choose a reason for hiding this comment

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

Left just one question. It's more about consistency across the workflow files.

.github/workflows/basic-qa.yml Show resolved Hide resolved
@GaryJones GaryJones merged commit e900d13 into develop Dec 14, 2023
46 checks passed
@GaryJones GaryJones deleted the feature/ghactions-improve-workflows branch December 14, 2023 11:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants