Refactor compatibility checker - #256
Conversation
…ware determination logic
github-actions
Bot
added
documentation
There was a problem hiding this comment.
🟡 Changes recommended
The CLI’s vendor-module default behaviour appears inconsistent with the service default/tests, and runScan() currently ignores its $thirdPartyOnly argument, both of which should be reconciled before approval.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Refactors the Hyvä compatibility checker/CLI surface to clarify option naming and simplify “has incompatibilities” into a more general “has issues”, while expanding Hyvä-aware module detection.
Changes:
- Rename results flag
hasIncompatibilities→hasIssuesacross checker, command, and tests. - Add/rename CLI flags (
--include-core,--exclude-vendor) and introduce early validation for conflicting scope options. - Extend module Hyvä-awareness detection to treat
hyva-themes.jsonas Hyvä-aware (with unit tests).
File summaries
| File | Description |
|---|---|
| tests/Unit/Service/Hyva/ModuleScannerTest.php | Adds coverage for hyva-themes.json Hyvä-awareness and adjusts isExists stubbing. |
| tests/Unit/Service/Hyva/CompatibilityCheckerTest.php | Updates assertions/helpers to use hasIssues. |
| tests/Unit/Console/Command/Hyva/CompatibilityCheckCommandTest.php | Updates expected results key and adds tests for new/renamed CLI options + conflict handling. |
| src/Service/Hyva/ModuleScanner.php | Introduces isHyvaAware() to include hyva-themes.json as a Hyvä-aware signal. |
| src/Service/Hyva/CompatibilityChecker.php | Renames results key to hasIssues and updates internal toggling accordingly. |
| src/Console/Command/Hyva/CompatibilityCheckCommand.php | Renames --include-vendor → --include-core, adds --exclude-vendor, adds option conflict validation, and switches to hasIssues. |
| docs/commands_reference.md | Documents renamed/added CLI options for hyva:check. |
| CHANGELOG.md | Updates listed options for the Hyvä compatibility check command. |
Review details
- Files reviewed: 8/8 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| $showAll = (bool) $input->getOption(self::OPTION_SHOW_ALL); | ||
| $thirdPartyOnly = (bool) $input->getOption(self::OPTION_THIRD_PARTY_ONLY); | ||
| $includeVendor = (bool) $input->getOption(self::OPTION_INCLUDE_VENDOR); | ||
| $includeCore = (bool) $input->getOption(self::OPTION_INCLUDE_CORE); | ||
| $excludeVendor = (bool) $input->getOption(self::OPTION_EXCLUDE_VENDOR); | ||
| $detailed = (bool) $input->getOption(self::OPTION_DETAILED); |
…lity check command
There was a problem hiding this comment.
🟡 Changes recommended
The “incompatible-only” interactive display mode is not currently enforced (the flag is not used to filter warning-only modules), which produces incorrect CLI output relative to the user’s selection.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 8/8 changed files
- Comments generated: 1
- Review effort level: Lite
| // default = show modules with any issues (critical or warnings) | ||
| $displayShowAll = $showAll && !$incompatibleOnly; | ||
|
|
||
| // Display results | ||
| $this->displayResults($results, $displayShowAll); |
No description provided.