Skip to content

Remove reliance on API#157

Draft
owenherbert-catalyst wants to merge 1 commit intomainfrom
ci-branches
Draft

Remove reliance on API#157
owenherbert-catalyst wants to merge 1 commit intomainfrom
ci-branches

Conversation

@owenherbert-catalyst
Copy link

This PR removes the dependency on the external Moodle API (download.moodle.org/api/1.3/updates.php) in the plugin CI workflow. Instead, all Moodle branch information is now sourced from a local JSON file (moodle_branches.json). This makes the workflow fully offline-capable and more stable.

You need to update moodle_branches.json whenever new stable Moodle releases are officially available and you want your CI to test them.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the CI matrix generation to stop querying the external Moodle updates API and instead derive Moodle branch/version breakpoints from a local moodle_branches.json, improving offline capability and reducing external dependency risk.

Changes:

  • Added moodle_branches.json as the local source of Moodle branch/version metadata.
  • Updated .github/actions/parse-version/script.php to read branch/version data from the local JSON instead of download.moodle.org.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
moodle_branches.json Introduces a local list of Moodle branches and their version breakpoints used for matrix selection.
.github/actions/parse-version/script.php Switches matrix filtering logic to consume the local branch/version list rather than the external Moodle API.
Comments suppressed due to low confidence (1)

.github/actions/parse-version/script.php:120

  • $upper from $plugin->supported is compared against $apiVersion['branch'] (e.g., 500 vs "5.0"), so $exists will never be true and main will be included even when the upper bound corresponds to an existing stable branch. Normalize both values (e.g., compare $upper to str_replace('.', '', $apiVersion['branch'])), or revert to checking the matrix entries as before.
        if ($entry['moodle-branch'] === 'main') {
            // If the upper range does NOT exist in the matrix, then assume the user wants the main to be tested.
            $exists = false;
            foreach ($updates as $apiVersion) {
                if ($apiVersion['branch'] == $upper) {
                    $exists = true;
                    break;
                }
            }

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +62 to +63
$branchesJson = file_get_contents("$workspace/ci/moodle_branches.json");
$updates = json_decode($branchesJson, true)['core'] ?? [];
Copy link

Copilot AI Feb 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reading and decoding moodle_branches.json has no failure handling: file_get_contents can return false and json_decode can return null, which will lead to warnings and an empty $updates set (silently altering the matrix). Consider failing fast with a clear message if the file is missing/unreadable or the JSON is invalid (e.g., JSON_THROW_ON_ERROR + try/catch).

Suggested change
$branchesJson = file_get_contents("$workspace/ci/moodle_branches.json");
$updates = json_decode($branchesJson, true)['core'] ?? [];
$branchesFilePath = "$workspace/ci/moodle_branches.json";
$branchesJson = file_get_contents($branchesFilePath);
if ($branchesJson === false) {
fwrite(STDERR, "Failed to read moodle branches file at {$branchesFilePath}" . PHP_EOL);
exit(1);
}
try {
$branchesData = json_decode($branchesJson, true, 512, JSON_THROW_ON_ERROR);
} catch (\JsonException $e) {
fwrite(STDERR, "Invalid JSON in moodle branches file at {$branchesFilePath}: " . $e->getMessage() . PHP_EOL);
exit(1);
}
$updates = $branchesData['core'] ?? [];

Copilot uses AI. Check for mistakes.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The file clearly exists...

Comment on lines +3 to +16
{ "branch": "3.8", "version": 2020082800 },
{ "branch": "3.9", "version": 2020120700 },
{ "branch": "3.10", "version": 2021041200 },
{ "branch": "3.11", "version": 2022080100 },
{ "branch": "4.0", "version": 2022080100 },
{ "branch": "4.1", "version": 2022112800 },
{ "branch": "4.2", "version": 2023042400 },
{ "branch": "4.3", "version": 2023100900 },
{ "branch": "4.4", "version": 2024042200 },
{ "branch": "4.5", "version": 2024100700 },
{ "branch": "5.0", "version": 2025041400 },
{ "branch": "5.1", "version": 2025100600 },
{ "branch": "5.2", "version": 2026040200 },
{ "branch": "main", "version": 9999999999 }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had removed support from EOL versions from the main matrix file.

It may be worth adding this to the matrix, instead? we could ues the full matrix to do this mapping instead? as that needs to be updated regurally instead

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

Comments