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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

CI: switch to GitHub Actions - step1 to 3 #18163

Merged
merged 3 commits into from Mar 8, 2022

Conversation

jrfnl
Copy link
Contributor

@jrfnl jrfnl commented Mar 7, 2022

Context

  • CI/QA is now run via GitHub Actions for the PHP code style, PHP linting and the Composer dependency security check

Summary

This PR can be summarized in the following changelog entry:

  • CI/QA is now run via GitHub Actions for the PHP code style, PHP linting and the Composer dependency security check

Relevant technical choices:

馃憠馃徎 This PR will be easiest to review by reviewing each commit individually.

CI: switch to GitHub Actions - step 1: code style

This commit:

  • Adds a GH Actions workflow for the CI code style check and to validate the composer.json file.
  • Removes those actions from the .travis.yml configuration.
  • Adds a few tweaks to the CS related functions in the Yoast\WP\SEO\Composer\Actions class.
  • Adds a "Build Status" badge in the Readme to use the results from this particular GH Actions run.

Notes:

  1. Builds will run on all pushes and on pull requests, except pushes to master.

  2. Builds can also be manually triggered.
    Note: manual triggering of builds has to be explicitly allowed. This is not a feature which is enabled by default.

  3. If a previous GH actions run for the same branch hadn't finished yet when the same branch is pushed again, the previous run will be cancelled.
    In Travis, this was an option on the "Settings" page - "Auto cancellation" -, which was turned on for most, if not all, Yoast repos. The concurrency configuration in the GHA script emulates the same behaviour.

  4. Composer dependency downloads will be cached for faster builds using a predefined GH action specifically created for this purpose.
    The alternative would be to handle the caching manually, which would add three extra steps to the script.
    Note: Caching works differently between Travis and GH Actions.
    On GH Actions, once a cache has been created, it can't be updated. It can only be replaced by a new cache with a different key.
    As the PHP version, the composer.json and a potential composer.lock hash are all part of the key used by the above mentioned action, this difference should not have a significant impact.
    Ref: https://docs.github.com/en/actions/advanced-guides/caching-dependencies-to-speed-up-workflows

  5. The CS check will display the (branch-based) results in the actions script output log, as well as display any violations found inline in the GitHub code view using the cs2pr tool.
    In contrast to some of the other repos, this repo uses a custom Composer script check-cs-threshold which potentially runs PHPCS twice:

    • Once to verify whether the CS error/warning counts are not above the predefined thresholds.
    • And if they are, it runs a separate PHPCS check just and only against the files changed in the current branch to get the actually relevant CS errors and warnings.

    This flow and the default output of it, is not compatible with showing the results of the PHPCS run inline in the code view on GitHub.
    To still be able to show the CS issues in-line, a few tweaks have been made to the functions in the Actions class.

    1. By default the Actions::check_cs_thresholds() runs both the thresholds check as well as the branch-based CS check. This remains the same for local runs, but when the function is run from within the GH Actions CI context (and therefore the CI environment variable has been set), the branch-based CS check is skipped.
    2. Instead, the branch-based CS check is run as a separate step from within the GH Actions script. This step will only be executed when the threshold check exits with a non-zero exit code.
    3. To make the results of the branch-based CS check usable by the cs2pr tool, a second report format needs to be used (checkstyle), however, the output of that report is not very human-friendly when viewed in the GitHub Actions transcript.
      To get round this and only when in CI, two reports will be generated:
      • The default full report which will be displayed in the GitHub Actions transcript.
      • The checkstyle report which will be written to file, with the file being subsequently used by the CS2PR action to display the results in the online code view as well.
    4. As a bonus extra, I've turned on colourized output for the PHPCS reports when shown in the GH Actions transcript.
      This is not turned on by default as it may not work on all OS platforms, but as we know it will work fine in GH Actions, we may as well use it.
  6. Finally, to allow for the above to work in combination with a shallow repo checkout, as is the default in GH Actions, we also need to make a few tweaks in the GH Actions script:

    1. We need to determine the base branch against which the git diff needs to be run for the "check changed files" CS run.
      For PRs, the branch against which the PR is pulled will be used. For pushes/manually run workflows, trunk will be used.
    2. We need to make sure that that base branch is available within the GH Actions environment by fetching it.
    3. And lastly, we need to pass the ref of the branch to the composer check-branch-cs command to ensure the diff is run against a ref which is recognized within the GH Actions environment.

Differences with the Travis implementation:

  • This check will now run on ALL pushes and pulls (except pushed to master).
    The branch filtering which was previously being applied in Travis has not been implemented for this script.
  • The composer validate command will now only be run against the PHP version used in the cs script (PHP 7.4), not against multiple PHP versions, but as the composer-install action does a composer validate since v2 under the hood, this shouldn't be an issue.
  • The PHPCS results cache which was previously used in Travis has not been implemented as with the different way caching works in GH Actions, it would actually work against us. As GH Actions is much faster than Travis anyway, this should not really be noticable in run time.

CI: switch to GitHub Actions - step 2: security check

This commit:

  • Adds a GH Actions workflow for the composer.lock dependency security check.
  • Removes all references to that action from the .travis.yml configuration.

Notes:

  1. Builds will run on all pushes and on pull requests.
  2. In addition to this, this workflow will run automatically every Monday at 6am against the default branch via a cron job.
    This is especially relevant for repositories which are not actively receiving PRs every week.
    Notes about workflows with cron jobs:
    1. If a repository is forked, workflows will also run in the fork. This means that with a cron job, those will also be run on forks.
      As that is not the intention, I've added a condition to prevent the cron job from running on forks, while still allowing the workflow to run on forks for other events.
      Note: this only applies to pre-existing forks. For new forks, scheduled jobs are disabled by default.
    2. There is one big downside to using cron jobs in workflows and that is that those workflows will automatically be disabled after 60 days of inactivity in a repo.
      In this context, an "inactive" repo means that there have been no commits to the repo within the last 60 days.
      The repo owner (organisation owner) will receive an email notification a few days before this is about to happen and can prevent the workflow from being disabled, but that does mean that for repos with low activity, this workflow will need to be kept active by acting on the email once every two months.
      If the workflow would still happen to get disabled, it can be re-enabled by anyone with commit access on the "Actions" -> "Security Check" page of the repo.
      Refs:
  3. Builds can also be manually triggered.
    Note: manual triggering of builds has to be explicitly allowed. This is not a feature which is enabled by default.
  4. If a previous GH actions run for the same branch hadn't finished yet when the same branch is pushed again, the previous run will be cancelled.
    In Travis, this was an option on the "Settings" page - "Auto cancellation" -, which was turned on for most, if not all, Yoast repos. The concurrency configuration in the GHA script emulates the same behaviour.
  5. The security check is run via a predefined action from the same maintainers as the security check tool which was previously used.
    In the documentation, it is suggested to potentially cache the vulnerabilities database, but knowing how caching works on GHA, I'm not so sure that's a good idea as in that case, chances of checking against an outdated database are high.
    Ref: https://github.com/marketplace/actions/the-php-security-checker

Differences with the Travis implementation:

  • This check will now run on ALL pushes and pulls.
    The branch filtering which was previously being applied in Travis has not been implemented for this script.
  • The check will now also run via the cron job.
  • No need to keep track of the current version number of the security check tool anymore as the predefined action will handle that.
  • As this check is now run in a separate workflow, there is no risk that the security check will run against a composer.lock file which has been updated during previous steps in the script. It will always run against the composer.lock file as committed into the repo, making the check much more reliable compared to how this check was run previously via Travis.

CI: switch to GitHub Actions - step 3: linting

This commit:

  • Adds a GH Actions workflow for the PHP lint CI check.
  • Removes all references to that action from the .travis.yml configuration.
  • Add a "Build Status" badge in the Readme to use the results from the GH Lint Action runs.
  • Adds two small tweaks to the lint script in the composer.json.

Notes:

  1. Builds will run on:
    • Select pushes using branch filtering similar to before.
    • All pull requests.
    • When manually triggered.
      Note: manual triggering of builds has to be explicitly allowed. This is not a feature which is enabled by default.
  2. If a previous GH actions run for the same branch hadn't finished yet when the same branch is pushed again, the previous run will be cancelled.
    In Travis, this was an option on the "Settings" page - "Auto cancellation" -, which was turned on for most, if not all, Yoast repos. The concurrency configuration in the GHA script emulates the same behaviour.
  3. The default ini settings used by the setup-php action are based on the php.ini-production configuration.
    This means that error_reporting=E_ALL & ~E_DEPRECATED & ~E_STRICT, display_errors is set to Off and zend.assertions is set to -1 (= do not compile).
    For the purposes of CI, especially for linting and testing code, I'd recommend running with zend.assertions=-1, error_reporting=-1 and display_errors=On to ensure all PHP notices are shown.
    Note: this only needs to be done for the PHP version used for the actual linting.
    Note: the defaults will be changed in the next major of setup-php to be based on the php.ini-develop configuration, but that may still be a while off.
    Refs:
  4. As the prefixing of the dependencies needs a higher PHP version, the setup-php action is used twice within this workflow.
    The first time, it sets up PHP 7.2 to allow for creating the vendor_prefixed directory.
    The second time, it sets up the PHP version on which we want to do the actual linting.
    Note: last time I checked, I found that within a workflow you can only switch PHP versions once, but that is sufficient for our purposes. (may have changed in the mean time)
  5. As the composer.lock file for this plugin contains dependencies which are locked at versions which are incompatible with the full range of PHP versions supported by the plugin, we need to do some juggling to allow the linting to work (on all supported PHP versions).
    Previously in Travis, an uncontrolled composer update would be run to get round this.
    Uncontrolled updates are generally not a good idea in CI as you may end up with passing tests based on updated dependencies, while in reality things would fail.
    So, for this workflow, I've chosen to approach this slightly differently (compared to Travis), but still tried to keep things as simple as possible.
    • First a full install is done on PHP 7.2. This will generate the vendor_prefixed directory, as well as set up all dependencies.
    • Next, the dependencies which are only needed for the prefixing are removed.
      This will remove a couple of dependencies which would automatically be loaded by Composer on loading of the autoload file (due to the use of autoload.files in those dependencies) and would trigger a parse error on PHP < 7.2 as they contain non-cross-version compatible code.
    • Now those incompatible dependencies are removed, we don't actually need to do a re-install of the dependencies on the target PHP version as all we'll be using is PHP-Parallel-Lint and PHP-Parallel-Lint is compatible with PHP 5.3 and higher, so the version installed on PHP 7.2 will work on all PHP versions on which we need to do the linting.
      It may be a bit counter-intuitive to do it like that, but it works and keeps the workflow as simple as possible, while still achieving the goal of the workflow. And we also avoid the uncontrolled update.
  6. While the setup-php action offers the ability to install the PHAR file for Parallel Lint, I've elected not to use that option as it would mean that we would not be able to use the composer lint script in the workflow, which would mean that the CLI arguments would have to be duplicated between the composer.json file and the lint.yml file.
    IMO, that would make this a typical point of failure where updates would be done in one, but not the other.
    If, at some point in the future, the Parallel Lint tool would start to support a config file for the CLI arguments, removing this point of failure, this choice can be (and should be) revisited.
  7. Composer dependency downloads will be cached for faster builds using a predefined GH action specifically created for this purpose.
    The alternative would be to handle the caching manually, which would add three extra steps to the script.
    Note: Caching works differently between Travis and GH Actions.
    On GH Actions, once a cache has been created, it can't be updated. It can only be replaced by a new cache with a different key.
    As the PHP version, the composer.json and a potential composer.lock hash are all part of the key used by the above mentioned action, this difference should not have a significant impact.
    Ref: https://docs.github.com/en/actions/advanced-guides/caching-dependencies-to-speed-up-workflows
  8. The Linting check will display the results inline in the GitHub code view using the cs2pr tool.

Differences with the Travis implementation:

  • There is a minor difference in the branch filtering being done for push events.
    Travis accepted PCRE regexes for the filtering. GH Actions uses glob patterns. The glob patterns now in place match the regexes as closely as possible, but are not an exact match to the "old" patterns. They should be sufficient for our purposes though.
  • The tag-based branch filter has not been added as that filter was only intended for use with the deploy stage.
  • Linting will now also be executed against PHP 8.1 and 8.2 (nightly).
    Linting runs against PHP 8.2 will be "allowed to fail" for the time being.
    Note: if any of the "allowed to fail" jobs actually fail, the workflow will show as successful, but there will still be a red x next to a PR.
    This is a known issue in GHA: Please support something like "allow-failure" for a given job聽actions/runner#2347
    There are work-arounds possible for this, however, the work-arounds would hide failures even more, meaning that chances are they won't be seen until they actually become a problem (no longer allowed to fail), which is too late.

Regarding the changes to the Composer lint scripts:

  • For the generic lint command, the vendor_prefixed directory is no longer excluded from the linting as the adjusted code does need to be cross-version compatible and safeguarding that PHP_Scoper doesn't introduce cross-version compatibility issues is a good thing.

Test instructions

Test instructions for the acceptance test before the PR gets merged

This PR can be acceptance tested by following these steps:

These workflows have been extensively tested already and need no further testing.

Aside from the builds run for this PR, you can find the results of various specific tests also on the "Actions" page.
For each job, it has been verified that the build(s) will actually show as failed when code has been altered to induce a fail condition.

@jrfnl jrfnl added yoast cs/qa changelog: non-user-facing Needs to be included in the 'Non-userfacing' category in the changelog labels Mar 7, 2022
@jrfnl jrfnl added this to the 18.4 milestone Mar 7, 2022
@jrfnl jrfnl requested a review from diedexx March 7, 2022 12:52
.github/workflows/cs.yml Outdated Show resolved Hide resolved
@jrfnl jrfnl force-pushed the JRF/switch-to-ghactions-step1-3 branch from a3f787b to baad852 Compare March 7, 2022 16:24
This commit:
* Adds a GH Actions workflow for the CI code style check and to validate the `composer.json` file.
* Removes those actions from the `.travis.yml` configuration.
* Adds a few tweaks to the CS related functions in the `Yoast\WP\SEO\Composer\Actions` class.
* Adds a "Build Status" badge in the Readme to use the results from this particular GH Actions run.

Notes:
1. Builds will run on all pushes and on pull requests, except pushes to `master`.
2. Builds can also be manually triggered.
    Note: manual triggering of builds has to be [explicitly allowed](https://github.blog/changelog/2020-07-06-github-actions-manual-triggers-with-workflow_dispatch/). This is not a feature which is enabled by default.
3. If a previous GH actions run for the same branch hadn't finished yet when the same branch is pushed again, the previous run will be cancelled.
    In Travis, this was an option on the "Settings" page - "Auto cancellation" -, which was turned on for most, if not all, Yoast repos. The `concurrency` configuration in the GHA script emulates the same behaviour.
4. Composer dependency downloads will be cached for faster builds using a [predefined GH action](https://github.com/marketplace/actions/install-composer-dependencies) specifically created for this purpose.
    The alternative would be to handle the caching manually, which would add three extra steps to the script.
    Note: Caching works differently between Travis and GH Actions.
    On GH Actions, once a cache has been created, it can't be updated. It can only be replaced by a new cache with a different key.
    As the PHP version, the `composer.json` and a potential `composer.lock` hash are all part of the key used by the above mentioned action, this difference should not have a significant impact.
    Ref: https://docs.github.com/en/actions/advanced-guides/caching-dependencies-to-speed-up-workflows
5. The CS check will display the (branch-based) results in the actions script output log, as well as display any violations found inline in the GitHub code view using the [cs2pr](https://github.com/staabm/annotate-pull-request-from-checkstyle) tool.
    In contrast to some of the other repos, this repo uses a custom Composer script `check-cs-threshold` which potentially runs PHPCS twice:
    - Once to verify whether the CS error/warning counts are not above the predefined thresholds.
    - And if they are, it runs a separate PHPCS check just and only against the files changed in the current branch to get the actually relevant CS errors and warnings.

    This flow and the default output of it, is not compatible with showing the results of the PHPCS run inline in the code view on GitHub.
    To still be able to show the CS issues in-line, a few tweaks have been made to the functions in the Actions class.
    1. By default the `Actions::check_cs_thresholds()` runs both the thresholds check as well as the branch-based CS check. This remains the same for local runs, but when the function is run from within the GH Actions `CI` context (and therefore the `CI` environment variable has been set), the branch-based CS check is skipped.
    2. Instead, the branch-based CS check is run as a separate step from within the GH Actions script. This step will only be executed when the threshold check exits with a non-zero exit code.
    3. To make the results of the branch-based CS check usable by the cs2pr tool, a second report format needs to be used (`checkstyle`), however, the output of that report is not very human-friendly when viewed in the GitHub Actions transcript.
        To get round this and only when in CI, two reports will be generated:
        - The default `full` report which will be displayed in the GitHub Actions transcript.
        - The `checkstyle` report which will be written to file, with the file being subsequently used by the CS2PR action to display the results in the online code view as well.
    4. As a bonus extra, I've turned on _colourized_ output for the PHPCS reports when shown in the GH Actions transcript.
        This is not turned on by default as it may not work on all OS platforms, but as we know it will work fine in GH Actions, we may as well use it.
6. Finally, to allow for the above to work in combination with a shallow repo checkout, as is the default in GH Actions, we also need to make a few tweaks in the GH Actions script:
    1. We need to determine the base branch against which the `git diff` needs to be run for the "check changed files" CS run.
        _For PRs, the branch against which the PR is pulled will be used. For pushes/manually run workflows, `trunk` will be used._
    2. We need to make sure that that base branch is available within the GH Actions environment by fetching it.
    3. And lastly, we need to pass the ref of the branch to the `composer check-branch-cs` command to ensure the diff is run against a ref which is recognized within the GH Actions environment.

 Differences with the Travis implementation:
* This check will now run on ALL pushes and pulls (except pushed to `master`).
    The branch filtering which was previously being applied in Travis has not been implemented for this script.
* The `composer validate` command will now only be run against the PHP version used in the `cs` script (PHP 7.4), not against multiple PHP versions, but as the `composer-install` action does a `composer validate` since v2 under the hood, this shouldn't be an issue.
* The PHPCS results cache which was previously used in Travis has not been implemented as with the different way caching works in GH Actions, it would actually work against us. As GH Actions is much faster than Travis anyway, this should not really be noticable in run time.
This commit:
* Adds a GH Actions workflow for the `composer.lock` dependency security check.
* Removes all references to that action from the `.travis.yml` configuration.

Notes:
1. Builds will run on all pushes and on pull requests.
2. In addition to this, this workflow will run automatically every Monday at 6am against the default branch via a cron job.
    This is especially relevant for repositories which are not actively receiving PRs every week.
    Notes about workflows with cron jobs:
    1. If a repository is forked, workflows will also run in the fork. This means that with a cron job, those will also be run on forks.
        As that is not the intention, I've added a condition to prevent the cron job from running on forks, while still allowing the workflow to run on forks for other events.
        Note: this only applies to pre-existing forks. For new forks, scheduled jobs are disabled by default.
    2. There is one big downside to using cron jobs in workflows and that is that those workflows will **_automatically be disabled after 60 days of inactivity in a repo_**.
        In this context, an "inactive" repo means that there have been no commits to the repo within the last 60 days.
        The repo owner (organisation owner) will receive an email notification a few days before this is about to happen and can prevent the workflow from being disabled, but that does mean that for repos with low activity, this workflow will need to be kept active by acting on the email once every two months.
        If the workflow would still happen to get disabled, it can be re-enabled by anyone with commit access on the "Actions" -> "Security Check" page of the repo.
        Refs:
        * https://docs.github.com/en/actions/managing-workflow-runs/disabling-and-enabling-a-workflow
        * https://github.community/t/do-disabled-scheduled-workflows-re-activate-on-repository-activity/160658
3. Builds can also be manually triggered.
    Note: manual triggering of builds has to be [explicitly allowed](https://github.blog/changelog/2020-07-06-github-actions-manual-triggers-with-workflow_dispatch/). This is not a feature which is enabled by default.
4. If a previous GH actions run for the same branch hadn't finished yet when the same branch is pushed again, the previous run will be cancelled.
    In Travis, this was an option on the "Settings" page - "Auto cancellation" -, which was turned on for most, if not all, Yoast repos. The `concurrency` configuration in the GHA script emulates the same behaviour.
5. The security check is run via a predefined action from the same maintainers as the security check tool which was previously used.
    In the documentation, it is suggested to potentially cache the vulnerabilities database, but knowing how caching works on GHA, I'm not so sure that's a good idea as in that case, chances of checking against an outdated database are high.
    Ref: https://github.com/marketplace/actions/the-php-security-checker

Differences with the Travis implementation:
* This check will now run on ALL pushes and pulls.
    The branch filtering which was previously being applied in Travis has not been implemented for this script.
* The check will now also run via the cron job.
* No need to keep track of the current version number of the security check tool anymore as the predefined action will handle that.
* As this check is now run in a separate workflow, there is no risk that the security check will run against a `composer.lock` file which has been updated during previous steps in the script. It will always run against the `composer.lock` file **_as committed into the repo_**, making the check much more reliable compared to how this check was run previously via Travis.
This commit:
* Adds a GH Actions workflow for the PHP lint CI check.
* Removes all references to that action from the `.travis.yml` configuration.
* Add a "Build Status" badge in the Readme to use the results from the GH `Lint` Action runs.
* Adds two small tweaks to the `lint` script in the `composer.json`.

Notes:
1. Builds will run on:
    - Select pushes using branch filtering similar to before.
    - All pull requests.
    - When manually triggered.
        Note: manual triggering of builds has to be [explicitly allowed](https://github.blog/changelog/2020-07-06-github-actions-manual-triggers-with-workflow_dispatch/). This is not a feature which is enabled by default.
2. If a previous GH actions run for the same branch hadn't finished yet when the same branch is pushed again, the previous run will be cancelled.
    In Travis, this was an option on the "Settings" page - "Auto cancellation" -, which was turned on for most, if not all, Yoast repos. The `concurrency` configuration in the GHA script emulates the same behaviour.
3. The default ini settings used by the `setup-php` action are based on the `php.ini-production` configuration.
    This means that `error_reporting=E_ALL & ~E_DEPRECATED & ~E_STRICT`, `display_errors` is set to `Off` and `zend.assertions` is set to `-1` (= do not compile).
    For the purposes of CI, especially for linting and testing code, I'd recommend running with `zend.assertions=-1`, `error_reporting=-1` and `display_errors=On` to ensure **all** PHP notices are shown.
    Note: this only needs to be done for the PHP version used for the actual linting.
    Note: the defaults will be changed in the next major of `setup-php` to be based on the `php.ini-develop` configuration, but that may still be a while off.
    Refs:
    * shivammathur/setup-php#450
    * shivammathur/setup-php#469
4. As the prefixing of the dependencies needs a higher PHP version, the `setup-php` action is used twice within this workflow.
    The first time, it sets up PHP 7.2 to allow for creating the `vendor_prefixed` directory.
    The second time, it sets up the PHP version on which we want to do the actual linting.
    Note: last time I checked, I found that within a workflow you can only switch PHP versions once, but that is sufficient for our purposes. (may have changed in the mean time)
5. As the `composer.lock` file for this plugin contains dependencies which are locked at versions which are incompatible with the full range of PHP versions supported by the plugin, we need to do some juggling to allow the linting to work (on all supported PHP versions).
    Previously in Travis, an uncontrolled `composer update` would be run to get round this.
    Uncontrolled updates are generally not a good idea in CI as you may end up with passing tests based on updated dependencies, while in reality things would fail.
    So, for this workflow, I've chosen to approach this slightly differently (compared to Travis), but still tried to keep things as simple as possible.
    - First a full install is done on PHP 7.2. This will generate the `vendor_prefixed` directory, as well as set up all dependencies.
    - Next, the dependencies which are only needed for the prefixing are removed.
        This will remove a couple of dependencies which would automatically be loaded by Composer on loading of the autoload file (due to the use of `autoload.files` in those dependencies) and would trigger a parse error on PHP < 7.2 as they contain non-cross-version compatible code.
    - Now those incompatible dependencies are removed, we **_don't_** actually need to do a re-`install` of the dependencies on the target PHP version as all we'll be using is PHP-Parallel-Lint and PHP-Parallel-Lint is compatible with PHP 5.3 and higher, so the version installed on PHP 7.2 will work on all PHP versions on which we need to do the linting.
    It may be a bit counter-intuitive to do it like that, but it works and keeps the workflow as simple as possible, while still achieving the goal of the workflow. And we also avoid the uncontrolled update.
6. While the `setup-php` action offers the ability to [install the PHAR file for Parallel Lint](https://github.com/shivammathur/setup-php#wrench-tools-support), I've elected not to use that option as it would mean that we would not be able to use the `composer lint` script in the workflow, which would mean that the CLI arguments would have to be duplicated between the `composer.json` file and the `lint.yml` file.
    IMO, that would make this a typical point of failure where updates would be done in one, but not the other.
    If, at some point in the future, the Parallel Lint tool would start to support a config file for the CLI arguments, removing this point of failure, this choice can be (and should be) revisited.
7. Composer dependency downloads will be cached for faster builds using a [predefined GH action](https://github.com/marketplace/actions/install-composer-dependencies) specifically created for this purpose.
    The alternative would be to handle the caching manually, which would add three extra steps to the script.
    Note: Caching works differently between Travis and GH Actions.
    On GH Actions, once a cache has been created, it can't be updated. It can only be replaced by a new cache with a different key.
    As the PHP version, the `composer.json` and a potential `composer.lock` hash are all part of the key used by the above mentioned action, this difference should not have a significant impact.
    Ref: https://docs.github.com/en/actions/advanced-guides/caching-dependencies-to-speed-up-workflows
8. The Linting check will display the results inline in the GitHub code view using the [cs2pr](https://github.com/staabm/annotate-pull-request-from-checkstyle) tool.

Differences with the Travis implementation:
* There is a minor difference in the branch filtering being done for `push` events.
    Travis accepted PCRE regexes for the filtering. GH Actions uses glob patterns. The glob patterns now in place match the regexes as closely as possible, but are not an exact match to the "old" patterns. They should be sufficient for our purposes though.
* The tag-based branch filter has not been added as that filter was only intended for use with the `deploy` stage.
* Linting will now also be executed against PHP 8.1 and 8.2 (nightly).
    Linting runs against PHP 8.2 will be "allowed to fail" for the time being.
    Note: if any of the "allowed to fail" jobs actually fail, the workflow will show as successful, but there will still be a red `x` next to a PR.
    This is a known issue in GHA: https://github.com/actions/toolkit/issues/399
    There are work-arounds possible for this, however, the work-arounds would hide failures even more, meaning that chances are they won't be seen until they actually become a problem (no longer allowed to fail), which is too late.

Regarding the changes to the Composer `lint` scripts:
* For the generic `lint` command, the `vendor_prefixed` directory is no longer excluded from the linting as the adjusted code **does** need to be cross-version compatible and safeguarding that PHP_Scoper doesn't introduce cross-version compatibility issues is a good thing.
@jrfnl jrfnl force-pushed the JRF/switch-to-ghactions-step1-3 branch from baad852 to 7eecf97 Compare March 7, 2022 16:26
@diedexx diedexx merged commit 9186e3b into trunk Mar 8, 2022
@diedexx diedexx deleted the JRF/switch-to-ghactions-step1-3 branch March 8, 2022 07:58
@igorschoester igorschoester modified the milestones: 18.4, 18.5 Mar 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog: non-user-facing Needs to be included in the 'Non-userfacing' category in the changelog yoast cs/qa
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants