Repository navigation
ci: Drop PHP 8.2, add PHP 8.5, and fix coding standards - #151
Merged
Merged
Conversation
PR #150 merged with a failing 'Checking coding standards' job and had dropped PHP 8.2 from the CI matrix. - Re-add php 8.2 to the integration-test matrix and lower the documented floor back to 8.2, adding the canonical PHP 8.3-only-syntax guards (#[Override], json_validate(), typed class constants, dynamic class constant fetch) to tests/Security/PhpCompatibilityTest.php so the 8.2 floor is actually enforced. The plugin source is already 8.2-clean. - Run php-cs-fixer over tests/bin/check-i18n-pot.php (added by #150) so 'composer run-script phpcsfixer' passes.
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The PHP 8.2 integration environment is incompatible with Cacti develop, and several compatibility guards miss valid PHP 8.3 syntax.
Review effort: Balanced
Findings: 1
Open (4)
What changed in this PR
Restores PHP 8.2 CI coverage and resolves coding-standard violations introduced by PR #150.
Changes:
- Adds PHP 8.2 to the integration matrix.
- Adds PHP 8.3 compatibility guards.
- Reformats the i18n template checker.
| File | Description |
|---|---|
.github/workflows/plugin-ci-workflow.yml |
Adds PHP 8.2 integration coverage. |
tests/Security/PhpCompatibilityTest.php |
Adds PHP 8.3 syntax checks. |
tests/bin/check-i18n-pot.php |
Applies coding-standard formatting. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Add a plugin-local composer.json documenting the supported PHP floor (>=8.2) to match the CI matrix. This is declarative metadata; CI runs Composer against the Cacti core tree, so this file is inert for the test harness.
Cacti develop requires PHP ^8.3, so 8.2 can never install its Composer dependencies (platform_check enforces >= 8.3.0). Remove 8.2 and add 8.5 instead; 8.5 satisfies the >= 8.3.0 floor and no locked dependency caps below it. Restore the documented 8.3 floor and bump composer.json to >=8.3.
TheWitness
changed the title
xmacan
approved these changes
Oct 5, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
php-cs-fixerovertests/bin/check-i18n-pot.php, which was added by ci: gate cacti.pot updates on i18n diff instead of regenerating #150 and failed thephpcsfixerjob).composer.jsondeclaring the supported floor"php": ">=8.3"(declarative; CI runs Composer against the Cacti core tree).tests/Security/PhpCompatibilityTest.php.Why
Cacti
developrequires"php": "^8.3"and its generatedplatform_check.phpenforcesPHP_VERSION_ID >= 80300. PHP 8.2 therefore can never install Cacti's Composer dependencies (the 8.2 job failed atInstall Cacti via CLIwith "Your Composer dependencies require a PHP version >= 8.3.0"). 8.5 satisfies the floor and no locked dependency caps below it, so it is a valid target to add.