From c558283f636bf40385bea09ebb70ed077d5be0e3 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Mon, 28 Apr 2025 01:57:17 +0200 Subject: [PATCH] GH Actions: fix cs annotations not showing 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. --- .github/workflows/test.yml | 10 +--------- .github/workflows/validate.yml | 26 ++++++++++++++++++++++++++ 2 files changed, 27 insertions(+), 9 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 83da62422a..6d5ae3102a 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -195,7 +195,6 @@ jobs: php-version: ${{ matrix.php }} ini-values: ${{ steps.set_ini.outputs.PHP_INI }} coverage: none - tools: cs2pr - name: "DEBUG: show libxml loaded version (php)" run: php -r 'echo "libxml loaded version = ", LIBXML_LOADED_VERSION, PHP_EOL;' @@ -240,14 +239,7 @@ jobs: - name: 'PHPCS: check code style without cache, no parallel' if: ${{ matrix.custom_ini == false }} - id: phpcs - run: > - php "bin/phpcs" --no-cache --parallel=1 - ${{ matrix.os == 'ubuntu-latest' && matrix.php == '8.4' && '--report-full --report-checkstyle=./phpcs-report.xml' || '' }} - - - name: Show PHPCS results in PR - if: ${{ always() && steps.phpcs.outcome == 'failure' && matrix.os == 'ubuntu-latest' && matrix.php == '8.4' }} - run: cs2pr ./phpcs-report.xml + run: php "bin/phpcs" --no-cache --parallel=1 - name: Download the PHPCS phar if: ${{ matrix.custom_ini == false }} diff --git a/.github/workflows/validate.yml b/.github/workflows/validate.yml index 60ca01ad0c..f898515291 100644 --- a/.github/workflows/validate.yml +++ b/.github/workflows/validate.yml @@ -17,6 +17,32 @@ concurrency: cancel-in-progress: true jobs: + phpcs: + name: Check PHP code style + runs-on: ubuntu-latest + + # Don't run the cronjob in this workflow on forks. + if: github.event_name != 'schedule' || (github.event_name == 'schedule' && github.repository_owner == 'PHPCSStandards') + + steps: + - name: Checkout code + uses: actions/checkout@v4 + + - name: Install PHP + uses: shivammathur/setup-php@v2 + with: + php-version: 'latest' + coverage: none + tools: cs2pr + + - name: Check PHP code style + id: phpcs + run: php "bin/phpcs" --report-full --report-checkstyle=./phpcs-report.xml + + - name: Show PHPCS results in PR + if: ${{ always() && steps.phpcs.outcome == 'failure' }} + run: cs2pr ./phpcs-report.xml + checkxml: name: Check XML files runs-on: ubuntu-latest