Repository navigation
ci: fix bootstrap-unit.php redeclare fatal, add coverage driver, document PHP 8.2 floor - #243
Merged
Merged
Conversation
…r to 8.2 - tests/bootstrap-unit.php: stop stubbing validateRequestVars() globally; monitor_controller.php declares it unconditionally, so the stub caused a Cannot redeclare fatal once a test loaded plugin source via loadPluginSource()/monitor_test_load(). - .github/workflows/plugin-ci-workflow.yml: add coverage: pcov to the setup-php step so the Pest --coverage-clover report has a driver to run against. - Document the PHP 8.2 floor (matrix is already 8.2-8.4) in copilot-instructions.md, the php-developer agent, and the hexagonal-monitor ADR, replacing stale PHP 8.1 references now that PHP 8.1 is no longer tested. - Verified apache2/libapache2-mod-php do not need to be re-added to the Install System Dependencies step: apache2 ships preinstalled on the ubuntu-latest runner image and shivammathur/setup-php configures the matching Apache PHP module for the selected matrix version.
Contributor
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
No unresolved review comments remain, and the CI and documentation updates are aligned.
Review effort: Lite
Findings: None
What changed in this PR
This PR fixes a test bootstrap redeclaration issue, enables PCOV coverage, and updates documentation for the PHP 8.2 minimum.
Changes:
- Removes the conflicting
validateRequestVars()stub. - Enables PCOV for Clover coverage generation.
- Updates PHP version guidance to 8.2+.
| File | Changes |
|---|---|
tests/bootstrap-unit.php |
Removes the conflicting validation stub. |
docs/architecture/0001-hexagonal-monitor.md |
Updates the PHP 8.2 baseline. |
.github/workflows/plugin-ci-workflow.yml |
Enables PCOV coverage. |
.github/copilot-instructions.md |
Documents the PHP 8.2 floor. |
.github/agents/php-developer.agent.md |
Updates PHP support guidance. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
xmacan
approved these changes
Sep 19, 2026
xmacan
approved these changes
Sep 19, 2026
xmacan
approved these changes
Sep 19, 2026
bmfmancini
approved these changes
Sep 20, 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.
Summary
Follow-up to the CI rebuild (#236), addressing review comments that are still valid against
develop(the default branch):tests/bootstrap-unit.php: removed the globalvalidateRequestVars()stub.monitor_controller.phpdeclares this function unconditionally, so once a test loads plugin source throughTestCase::loadPluginSource()/monitor_test_load(), the stub causes aCannot redeclare validateRequestVars()fatal. No test currently calls the stub directly, so removing it is safe..github/workflows/plugin-ci-workflow.yml: addedcoverage: pcovto thesetup-phpstep. The Pest run already requests--coverage-clover, but no coverage driver was installed, so the coverage report could silently fail to generate.['8.2', '8.3', '8.4']) in.github/copilot-instructions.md,.github/agents/php-developer.agent.md, anddocs/architecture/0001-hexagonal-monitor.md, all of which still referenced PHP 8.1.Not changed
apache2ships preinstalled on theubuntu-latestGitHub-hosted runner image, andshivammathur/setup-phpconfigures the matchinglibapache2-mod-phpmodule for the selected matrix PHP version. Re-adding an explicitapt-get install apache2 libapache2-mod-phpstep would be redundant.