initial attempt to move ci checks to their own jobs #155
Conversation
There was a problem hiding this comment.
Pull request overview
This PR restructures the CI workflow by splitting individual checks (phpunit, codechecker, phplint, etc.) into separate jobs instead of running them sequentially within a single setup job. The changes introduce a canonical workspace setup that is shared across jobs via artifact upload/download, and uses YAML anchors for common configuration.
Changes:
- Refactored CI checks from a single
setupjob into multiple independent jobs (codechecker, phplint, validate, savepoints, mustache, grunt, phpcpd, phpmd, behat, phpdoc) - Added a
setup-singlejob that creates a canonical workspace artifact shared across all check jobs - Simplified the plugin setup action by removing check execution logic and check-specific inputs, replacing them with environment configuration inputs
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 14 comments.
| File | Description |
|---|---|
| .github/workflows/ci.yml | Splits monolithic setup job into individual check jobs, adds canonical workspace setup, updates deprecated GitHub Actions output syntax |
| .github/plugin/setup/action.yml | Removes all check execution steps and their related inputs, simplifies to only handle environment setup with matrix values replaced by direct inputs |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| <<: *ci_setup | ||
| name: Codechecker | ||
| if: ${{ inputs.disable_phpcs != true }} | ||
| steps: |
There was a problem hiding this comment.
The steps key at line 278 overrides the inherited steps from the YAML anchor *ci_setup, which includes downloading the canonical workspace artifact. This means the codechecker job won't have access to the workspace setup. The download step should be included in this job's steps array.
| steps: | |
| steps: | |
| - name: Download canonical workspace | |
| uses: actions/download-artifact@v3 | |
| with: | |
| name: canonical-workspace | |
| path: ${{ github.workspace }} |
| steps: | ||
| - name: Run phplint |
There was a problem hiding this comment.
The steps key overrides the inherited steps from the YAML anchor *ci_setup. The workspace download step needs to be included in this job's steps array to access the canonical workspace.
| steps: | ||
| - name: Run validate |
There was a problem hiding this comment.
The steps key overrides the inherited steps from the YAML anchor *ci_setup. The workspace download step needs to be included in this job's steps array to access the canonical workspace.
| steps: | ||
| - name: Run savepoints |
There was a problem hiding this comment.
The steps key overrides the inherited steps from the YAML anchor *ci_setup. The workspace download step needs to be included in this job's steps array to access the canonical workspace.
.github/workflows/ci.yml
Outdated
| steps: | ||
| - name: Run behat |
There was a problem hiding this comment.
The steps key overrides the inherited steps from the YAML anchor *ci_setup. The workspace download step needs to be included in this job's steps array to access the canonical workspace.
| steps: | ||
| - name: Run phpdoc |
There was a problem hiding this comment.
The steps key overrides the inherited steps from the YAML anchor *ci_setup. The workspace download step needs to be included in this job's steps array to access the canonical workspace.
| needs: | ||
| - pre_job | ||
| - prepare_matrix | ||
| - matrix | ||
| - setup-single | ||
| - codechecker | ||
| - phplint | ||
| - validate | ||
| - savepoints | ||
| - mustache | ||
| - grunt | ||
| - phpcpd | ||
| - phpmd | ||
| - behat | ||
| - phpdoc |
There was a problem hiding this comment.
The release job requires all check jobs to complete successfully, but some of these jobs are conditionally skipped (e.g., phpmd only runs if enabled, codechecker/phplint/etc. only run if not disabled). If a job is skipped, GitHub Actions treats it as a failure for dependency purposes. Consider using if: always() conditions or needs with conditional success checks to handle optional jobs properly.
.github/plugin/setup/action.yml
Outdated
| if: ${{ matrix.moodle-branch == 'MOODLE_32_STABLE' || matrix.moodle-branch == 'MOODLE_33_STABLE' }} | ||
| if: ${{ inputs.moodle_branch == 'MOODLE_32_STABLE' || inputs.moodle_branch == 'MOODLE_33_STABLE' }} | ||
| run: | | ||
| echo "::set-output name=COMPOSER_VERSION::--1" |
There was a problem hiding this comment.
This uses the deprecated set-output command. Update to the newer syntax: echo \"COMPOSER_VERSION=--1\" >> $GITHUB_OUTPUT
| echo "::set-output name=COMPOSER_VERSION::--1" | |
| echo "COMPOSER_VERSION=--1" >> $GITHUB_OUTPUT |
.github/workflows/ci.yml
Outdated
| uses: actions/upload-artifact@v3 | ||
| with: | ||
| name: canonical-workspace | ||
| path: ${{ github.workspace }} |
There was a problem hiding this comment.
Uploading the entire workspace as an artifact may be inefficient and could include unnecessary files. Consider uploading only the essential directories (e.g., moodle installation, plugin code) to reduce artifact size and improve upload/download times.
| path: ${{ github.workspace }} | |
| path: | | |
| ${{ github.workspace }}/moodle | |
| ${{ github.workspace }}/plugin |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| needs: | ||
| - pre_job | ||
| - prepare_matrix | ||
| - matrix | ||
| - setup-single | ||
| - codechecker | ||
| - phplint | ||
| - validate | ||
| - savepoints | ||
| - mustache | ||
| - grunt | ||
| - phpcpd | ||
| - phpmd | ||
| - behat | ||
| - phpdoc |
There was a problem hiding this comment.
The release job now requires all check jobs to complete, but many of these jobs have conditional execution based on input flags. If a check is disabled, the release job will not run because the disabled job won't execute. These dependencies should be made conditional or use a pattern that allows the release job to proceed even when some checks are skipped.
Co-authored-by: Copilot <[email protected]>
So far un-tested, creating the PR now, will need to refrence this branch in some repositories to test it properly