Allow for required GHA job statuses to be reported so that all PR checks have a status #6378
Conversation
pierlon
force-pushed
the
make-required-pr-checks-optional
branch
from
d2cc311 to
8689607
Compare
pierlon
changed the title
pierlon
changed the title
pierlon
added
the
Infrastructure
|
Plugin builds for 1ba9c2e are ready 🛎️!
|
| name: 'Pre run' | ||
| runs-on: ubuntu-latest | ||
| outputs: | ||
| changed-file-count: ${{ steps.determine-file-count.outputs.count }} |
There was a problem hiding this comment.
Idea: what if there was a changed-php-file-count, changed-css-file-count, and changed-js-file-count? Then lint-css could, for example, only run if needs.pre-run.outputs.changed-css-file-count > 0. Same for the others. This would speed up the runs a bit.
There was a problem hiding this comment.
Me gusta. I'll make the change.
Co-authored-by: Weston Ruter <[email protected]>
pierlon
force-pushed
the
make-required-pr-checks-optional
branch
from
1b114a8 to
2f8e444
Compare
| @@ -258,6 +315,8 @@ jobs: | |||
|
|
|||
| e2e-test-js: | |||
| name: 'E2E test: JS' | |||
| needs: pre-run | |||
| if: needs.pre-run.outputs.changed-file-count > 0 | |||
There was a problem hiding this comment.
Not sure if we'd want to make this check for modified files too strict, as this touches multiple sections of the plugin.
| @@ -334,6 +393,8 @@ jobs: | |||
| unit-test-php: | |||
| name: "Unit test${{ matrix.coverage && ' (with coverage)' || '' }}: PHP ${{ matrix.php }}, WP ${{ matrix.wp }}" | |||
| runs-on: ubuntu-latest | |||
| needs: pre-run | |||
There was a problem hiding this comment.
Same sentiment here.
| @@ -504,6 +565,8 @@ jobs: | |||
|
|
|||
| feature-test-php: | |||
| name: "Feature test${{ matrix.coverage && ' (with coverage)' || '' }}: PHP ${{ matrix.php }}, WP ${{ matrix.wp }}" | |||
| needs: pre-run | |||
| if: needs.pre-run.outputs.changed-file-count > 0 | |||
There was a problem hiding this comment.
Same sentiment here.
| @@ -138,7 +202,8 @@ jobs: | |||
| unit-test-php: | |||
| name: 'Unit test: PHP (v${{ matrix.php }})' | |||
| runs-on: ubuntu-latest | |||
| needs: [lint-css, lint-js, lint-php] | |||
| needs: pre-run | |||
| if: needs.pre-run.outputs.changed-file-count > 0 | |||
There was a problem hiding this comment.
Same sentiment here.
There was a problem hiding this comment.
The concern being that a PHPUnit test may load a CSS file or a JS file?
| uses: GoogleCloudPlatform/github-actions/setup-gcloud@master | ||
| uses: google-github-actions/setup-gcloud@master |
There was a problem hiding this comment.
The previous action was deprecated, see https://github.com/google-github-actions/setup-gcloud/blob/b94035a6aac71cdacf6e682bbbef2e927960f8ac/setup-gcloud/README.md.
westonruter
added
the
Changelogged
Summary
One issue we've been having with required GHA jobs is that whenever their statuses are not reported (due to a workflow not running because a modified file does not match the configured
pathsorpaths-ignoreglob patterns, for example), it prevents PR auto-merging from occurring as all the required PR checks have not reported a status. Instead, these required GHA jobs are forever left in a pending state for which a status will never be reported. An example:This PR attempts to resolve this issue by removing these
pathsandpaths-ignoreglob patterns from thebuild-test-measure.ymlandqa-integrate.ymlworkflows, and instead adds a "pre-run" job that replicates the feature. The job retrieves the list of modified files from the PR (or previous commit if not a PR), and then uses a pre-defined regex pattern to detect the modified files related to the workflow. If there is at least one match, the workflow run continues, and if not, the rest of the workflow is skipped and the required jobs will be given a "skipped" status.Checklist