GH Actions: fix cs annotations not showing #1063
Merged
Conversation
This file contains hidden or 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
As things are, the CS check for the PHPCS codebase is run in the `test` workflow as an integration test and will show inline annotations about code style issues in PRs. These annotations are intended to help contributors identify issues with their PR.
As the `test` workflow is run against a large range of PHP versions and OSes, this showing of the annotations was limited to PHP 8.4 + Ubuntu to prevent the contributor seeing a wall of duplicate annotations when issues would be found (one for each test build).
However, the `test` workflow also uses the "fail-fast" option, cancelling the builds on the first failure, which means the PHP 8.4/Ubuntu build may have been cancelled before it was run, in which case, the inline CS annotations don't show at all.
This means the current setup is counter-productive as in most cases, PR annotations will not show, even when they should be shown.
This commit intends to fix this by adding a separate CS run against PHP "latest" to the `validate` workflow.
This CS run in the `validate` workflow will now trigger the showing of annotations, and as this job is stand-alone, will not be subject to cancellation due to other builds failing.
The code related to showing the annotations has been removed from the `test` workflow.
Yes, this does mean the CS check will have a semi-duplicated run in the `test` workflow.
I do not see this as a problem as:
1. The _intention_ behind that run is different ("integration test" vs "cs check").
2. The CS run in the `test` workflow is run with different settings - `--no-cache --parallel=1` -, meaning more variations of PHPCS CLI args are tested.
|
Note: branch protection should be updated once this PR has been merged. |
jrfnl
deleted the
feature/ghactions-fix-inline-annotations-for-cs-errors
branch
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.
Description
As things are, the CS check for the PHPCS codebase is run in the
testworkflow as an integration test and will show inline annotations about code style issues in PRs. These annotations are intended to help contributors identify issues with their PR.As the
testworkflow is run against a large range of PHP versions and OSes, this showing of the annotations was limited to PHP 8.4 + Ubuntu to prevent the contributor seeing a wall of duplicate annotations when issues would be found (one for each test build).However, the
testworkflow also uses the "fail-fast" option, cancelling the builds on the first failure, which means the PHP 8.4/Ubuntu build may have been cancelled before it was run, in which case, the inline CS annotations don't show at all.This means the current setup is counter-productive as in most cases, PR annotations will not show, even when they should be shown.
This commit intends to fix this by adding a separate CS run against PHP "latest" to the
validateworkflow.This CS run in the
validateworkflow will now trigger the showing of annotations, and as this job is stand-alone, will not be subject to cancellation due to other builds failing.The code related to showing the annotations has been removed from the
testworkflow.Yes, this does mean the CS check will have a semi-duplicated run in the
testworkflow. I do not see this as a problem as:testworkflow is run with different settings ---no-cache --parallel=1-, meaning more variations of PHPCS CLI args are tested.Suggested changelog entry
N/A