ci: add Moodle Plugin CI workflow - #3
Conversation
|
No actionable comments were generated in the recent review. 🎉 Recent review infoRun configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughSummary by CodeRabbit
WalkthroughAdds ChangesMoodle Plugin CI Workflow
|
There was a problem hiding this comment.
Actionable comments posted: 3
Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/moodle-ci.yaml:
- Around line 30-42: The CI matrix currently only includes PHP 8.4 and has an
exclude rule that prevents MOODLE_405_STABLE from being tested with PHP 8.4,
which means the minimum supported Moodle branch declared in version.php is never
exercised by the CI. Add another PHP version to the php array that is compatible
with MOODLE_405_STABLE (such as an earlier PHP version), and then either remove
the exclude rule or modify it to only exclude incompatible PHP and Moodle branch
combinations, allowing at least one combination to test the minimum supported
Moodle branch.
- Line 45: GitHub Actions at lines 45, 50, and 119 are using tag-based
references (such as `@v4`) instead of immutable full-length commit SHAs, which
creates a supply-chain security risk. For each of these lines, replace the
tag-based reference with the corresponding resolved full commit SHA. For
example, actions/checkout@v4 should be replaced with actions/checkout@ followed
by the full 40-character commit SHA that `@v4` currently resolves to. This applies
to all three instances where `@v`* tags are used.
- Line 53: The `matrix.extensions` reference is undefined because the extensions
key is not defined in the matrix configuration (which only contains php,
moodle-branch, and database). Either add extensions as a new matrix key with
appropriate values, or remove the reference to matrix.extensions and use a
static value if extension configuration is not matrix-dependent. Additionally,
the `join(matrix.*, ', ')` expression on the other line uses invalid syntax that
cannot filter matrix object values; replace this with valid GitHub Actions
syntax such as referencing a specific matrix key or restructure the logic to
properly join the intended values.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
There was a problem hiding this comment.
Actionable comments posted: 1
Nitpick comments (3)
.github/workflows/moodle-ci.yaml (3)
17-17: ⚡ Quick winConsider pinning the MariaDB image to a specific minor version.
The
mariadb:10tag references the latest patch release in the 10.x series, which may introduce non-deterministic behaviour across builds. Pinning to a specific minor version (e.g.,mariadb:10.11) would improve reproducibility.Proposed fix
- image: mariadb:10 + image: mariadb:10.11Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/moodle-ci.yaml at line 17, The MariaDB image tag in the CI workflow is using only the major version (mariadb:10), which pulls the latest patch release and causes non-deterministic builds. Pin the image to a specific minor version (such as mariadb:10.11) instead of mariadb:10 to ensure reproducible and consistent builds across different CI runs. This change applies to the image specification in the MariaDB service configuration.
88-93: ⚖️ Poor tradeoffConsider whether PHPDoc failures should block CI.
Similar to PHPMD,
continue-on-error: trueallows PHPDoc checker to fail without failing the job. Whilst--max-warnings 0enforces strict checking when it runs, failures won't prevent merges.Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/moodle-ci.yaml around lines 88 - 93, The Moodle PHPDoc Checker step has a contradiction: it uses --max-warnings 0 to enforce strict checking, but also has continue-on-error: true which allows the step to fail without blocking the job. Decide whether PHPDoc failures should block CI merges. If yes, remove the continue-on-error: true directive from the Moodle PHPDoc Checker step to make failures fail the job. If no, adjust the --max-warnings threshold or remove the strict enforcement. The current configuration sends mixed signals about the importance of PHPDoc compliance.
79-82: ⚖️ Poor tradeoffConsider whether PHPMD failures should block CI.
The
continue-on-error: truesetting allows PHPMD to fail without failing the job, which reduces CI strictness. This may be intentional if PHPMD produces false positives, but it means potential code quality issues won't block merges.Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/moodle-ci.yaml around lines 79 - 82, The PHP Mess Detector step in the moodle-ci.yaml workflow has `continue-on-error: true` set, which allows PHPMD to fail without blocking the CI job. Determine if this is intentional based on your project's code quality standards: either remove the `continue-on-error: true` line from the PHP Mess Detector step to make PHPMD failures block CI and enforce code quality checks, or keep it if PHPMD produces acceptable false positives that shouldn't block merges. Make an explicit decision and ensure it aligns with your project's CI strictness requirements.
Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/moodle-ci.yaml:
- Line 56: The extensions list in the GitHub Actions workflow configuration is
missing the required pdo_sqlite PHP extension. Update the extensions parameter
that currently contains "mysqli,pdo_mysql" to include pdo_sqlite by adding it to
the comma-separated list, ensuring the plugin's core export functionality and
tests can run successfully without encountering missing extension errors.
---
Nitpick comments:
In @.github/workflows/moodle-ci.yaml:
- Line 17: The MariaDB image tag in the CI workflow is using only the major
version (mariadb:10), which pulls the latest patch release and causes
non-deterministic builds. Pin the image to a specific minor version (such as
mariadb:10.11) instead of mariadb:10 to ensure reproducible and consistent
builds across different CI runs. This change applies to the image specification
in the MariaDB service configuration.
- Around line 88-93: The Moodle PHPDoc Checker step has a contradiction: it uses
--max-warnings 0 to enforce strict checking, but also has continue-on-error:
true which allows the step to fail without blocking the job. Decide whether
PHPDoc failures should block CI merges. If yes, remove the continue-on-error:
true directive from the Moodle PHPDoc Checker step to make failures fail the
job. If no, adjust the --max-warnings threshold or remove the strict
enforcement. The current configuration sends mixed signals about the importance
of PHPDoc compliance.
- Around line 79-82: The PHP Mess Detector step in the moodle-ci.yaml workflow
has `continue-on-error: true` set, which allows PHPMD to fail without blocking
the CI job. Determine if this is intentional based on your project's code
quality standards: either remove the `continue-on-error: true` line from the PHP
Mess Detector step to make PHPMD failures block CI and enforce code quality
checks, or keep it if PHPMD produces acceptable false positives that shouldn't
block merges. Make an explicit decision and ensure it aligns with your project's
CI strictness requirements.
🪄 Autofix (Beta)
❌ Autofix failed (check again to retry)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Autofix skipped. No unresolved CodeRabbit review comments with fix instructions found. |
Adds GitHub Actions CI workflow for automated testing (PHP lint, code checker, PHPUnit, Behat). Only PHP 8.4 is used as this is a simple plugin.