-
-
Notifications
You must be signed in to change notification settings - Fork 28
[#2014] Allow to select modules in the installer. #2108
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[#2014] Allow to select modules in the installer. #2108
Conversation
WalkthroughAdds a new Modules prompt handler that discovers drupal/* contributed modules from a template composer.json, presents a multi-select defaulting to all, removes unselected modules and related artifacts, integrates the prompt into PromptManager, updates tests, wraps JSON parsing with error handling, and adds comment/marker delimiters and lint script tweaks. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant P as PromptManager
participant M as ModulesHandler
participant J as JsonManipulator
participant C as composer.json
participant S as ProvisionScript
participant F as Feature/Test files
U->>P: start installer
P->>M: discover()
M->>J: fromFile(template composer.json)
alt valid JSON
J-->>M: module list
else invalid JSON
J-->>M: NULL (fallback to defaults)
end
M-->>P: options() (multiselect, default all)
P-->>U: present Modules multi-select
U->>P: submit selections
P->>M: process()
rect rgb(230,250,230)
Note over M: For each unselected module
M->>C: remove drupal/<module> entries
M->>S: remove provision drush install lines / includes
M->>F: remove Behat features/tests and MODULE_* tokens
end
M-->>P: processing complete
P-->>U: continue install
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (71)
.vortex/docs/static/img/installer.svgis excluded by!**/*.svg.vortex/installer/composer.lockis excluded by!**/*.lock.vortex/installer/tests/Fixtures/install/_baseline/scripts/custom/provision-10-example.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_admin_toolbar/composer.jsonis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_admin_toolbar/scripts/custom/provision-10-example.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_coffee/composer.jsonis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_coffee/scripts/custom/provision-10-example.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_config_split/composer.jsonis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_config_split/scripts/custom/provision-10-example.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_config_split/tests/phpunit/Drupal/EnvironmentSettingsTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_config_split/tests/phpunit/Drupal/SwitchableSettingsTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_config_split/web/sites/default/includes/modules/-settings.config_split.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_config_update/composer.jsonis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_config_update/scripts/custom/provision-10-example.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_environment_indicator/composer.jsonis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_environment_indicator/scripts/custom/provision-10-example.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_environment_indicator/tests/phpunit/Drupal/EnvironmentSettingsTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_environment_indicator/tests/phpunit/Drupal/SwitchableSettingsTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_environment_indicator/web/sites/default/includes/modules/-settings.environment_indicator.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_pathauto/composer.jsonis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_pathauto/scripts/custom/provision-10-example.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_redirect/composer.jsonis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_redirect/scripts/custom/provision-10-example.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_robotstxt/composer.jsonis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_robotstxt/scripts/custom/provision-10-example.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_robotstxt/tests/behat/features/-robotstxt.featureis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_robotstxt/tests/phpunit/Drupal/EnvironmentSettingsTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_robotstxt/web/sites/default/includes/modules/-settings.robotstxt.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_seckit/composer.jsonis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_seckit/tests/phpunit/Drupal/EnvironmentSettingsTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_seckit/web/sites/default/includes/modules/-settings.seckit.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_seckit_shield_stage_file_proxy/.envis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_seckit_shield_stage_file_proxy/composer.jsonis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_seckit_shield_stage_file_proxy/docker-compose.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_seckit_shield_stage_file_proxy/scripts/custom/provision-10-example.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_seckit_shield_stage_file_proxy/tests/phpunit/Drupal/EnvironmentSettingsTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_seckit_shield_stage_file_proxy/tests/phpunit/Drupal/SwitchableSettingsTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_seckit_shield_stage_file_proxy/web/sites/default/includes/modules/-settings.seckit.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_seckit_shield_stage_file_proxy/web/sites/default/includes/modules/-settings.shield.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_seckit_shield_stage_file_proxy/web/sites/default/includes/modules/-settings.stage_file_proxy.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_shield/.envis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_shield/composer.jsonis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_shield/docker-compose.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_shield/scripts/custom/provision-10-example.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_shield/tests/phpunit/Drupal/EnvironmentSettingsTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_shield/tests/phpunit/Drupal/SwitchableSettingsTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_shield/web/sites/default/includes/modules/-settings.shield.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_stage_file_proxy/composer.jsonis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_stage_file_proxy/scripts/custom/provision-10-example.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_stage_file_proxy/tests/phpunit/Drupal/SwitchableSettingsTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_stage_file_proxy/web/sites/default/includes/modules/-settings.stage_file_proxy.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_none/.envis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_none/composer.jsonis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_none/docker-compose.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_none/scripts/custom/provision-10-example.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_none/tests/behat/features/-robotstxt.featureis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_none/tests/phpunit/Drupal/EnvironmentSettingsTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_none/tests/phpunit/Drupal/SwitchableSettingsTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_none/web/sites/default/includes/modules/-settings.config_split.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_none/web/sites/default/includes/modules/-settings.environment_indicator.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_none/web/sites/default/includes/modules/-settings.robotstxt.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_none/web/sites/default/includes/modules/-settings.seckit.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_none/web/sites/default/includes/modules/-settings.shield.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_none/web/sites/default/includes/modules/-settings.stage_file_proxy.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/names/scripts/custom/provision-10-example.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/non_interactive_config_file/scripts/custom/provision-10-example.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/non_interactive_config_string/scripts/custom/provision-10-example.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/services_no_clamav/scripts/custom/provision-10-example.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/services_no_redis/scripts/custom/provision-10-example.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/services_no_solr/scripts/custom/provision-10-example.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/services_none/scripts/custom/provision-10-example.shis excluded by!.vortex/installer/tests/Fixtures/**
📒 Files selected for processing (12)
.env(1 hunks).vortex/installer/composer.json(2 hunks).vortex/installer/src/Prompts/Handlers/ModulePrefix.php(1 hunks).vortex/installer/src/Prompts/Handlers/Modules.php(1 hunks).vortex/installer/src/Prompts/PromptManager.php(6 hunks).vortex/installer/src/Utils/JsonManipulator.php(1 hunks).vortex/installer/tests/Functional/Handlers/ModulesInstallTest.php(1 hunks).vortex/installer/tests/Unit/Handlers/AbstractPromptManagerTestCase.php(3 hunks).vortex/installer/tests/Unit/Handlers/ModulesPromptManagerTest.php(1 hunks)docker-compose.yml(1 hunks)scripts/custom/provision-10-example.sh(1 hunks)tests/phpunit/Drupal/SwitchableSettingsTest.php(6 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-09-13T04:14:41.765Z
Learnt from: AlexSkrypnyk
Repo: drevops/vortex PR: 2011
File: .vortex/tests/phpunit/Traits/Steps/StepAhoyTrait.php:20-23
Timestamp: 2025-09-13T04:14:41.765Z
Learning: In .vortex/tests/phpunit PHPUnit tests using cmd/cmdFail helpers, patterns in the expected output arrays support '*' markers for positive assertions (text must be present) and '!' markers for negative assertions (text must NOT be present). The '*' markers are NOT literal text matches but rather indicate expected presence.
Applied to files:
.vortex/installer/tests/Unit/Handlers/ModulesPromptManagerTest.php.vortex/installer/tests/Unit/Handlers/AbstractPromptManagerTestCase.php
📚 Learning: 2025-06-01T08:10:15.903Z
Learnt from: AlexSkrypnyk
Repo: drevops/vortex PR: 1693
File: .vortex/installer/src/Prompts/Handlers/Internal.php:52-52
Timestamp: 2025-06-01T08:10:15.903Z
Learning: The File utility class in .vortex/installer/src/Utils/File.php extends AlexSkrypnyk\File\File, and methods like collapseRepeatedEmptyLines() are available from this external parent class dependency.
Applied to files:
.vortex/installer/src/Utils/JsonManipulator.php.vortex/installer/composer.json
🧬 Code graph analysis (6)
.vortex/installer/tests/Unit/Handlers/ModulesPromptManagerTest.php (3)
.vortex/installer/src/Prompts/Handlers/Modules.php (2)
Modules(10-179)getAvailableModules(124-138).vortex/installer/src/Utils/Config.php (1)
Config(14-126).vortex/installer/tests/Unit/Handlers/AbstractPromptManagerTestCase.php (5)
AbstractPromptManagerTestCase(50-276)getExpectedDefaults(120-151)getExpectedInstalled(156-166)stubVortexProject(262-268)stubComposerJsonDependencies(230-252)
.vortex/installer/tests/Functional/Handlers/ModulesInstallTest.php (4)
.vortex/installer/src/Prompts/Handlers/Modules.php (2)
Modules(10-179)getAvailableModules(124-138).vortex/installer/src/Prompts/PromptManager.php (2)
PromptManager(61-625)makeEnvName(443-445).vortex/installer/src/Utils/Env.php (2)
Env(7-241)put(24-26).vortex/installer/tests/Functional/FunctionalTestCase.php (1)
assertSutNotContains(130-145)
.vortex/installer/src/Prompts/PromptManager.php (2)
.vortex/installer/src/Prompts/Handlers/Modules.php (1)
Modules(10-179).vortex/installer/src/Prompts/Handlers/AbstractHandler.php (2)
id(52-62)options(95-97)
.vortex/installer/tests/Unit/Handlers/AbstractPromptManagerTestCase.php (3)
.vortex/installer/src/Prompts/Handlers/Modules.php (2)
Modules(10-179)getAvailableModules(124-138).vortex/installer/src/Prompts/Handlers/AbstractHandler.php (1)
id(52-62).vortex/installer/src/Prompts/Handlers/HandlerInterface.php (1)
id(19-19)
scripts/custom/provision-10-example.sh (1)
scripts/vortex/provision.sh (2)
task(71-71)drush(77-77)
.vortex/installer/src/Prompts/Handlers/Modules.php (3)
.vortex/installer/src/Utils/File.php (2)
File(10-121)removeTokenAsync(93-100).vortex/installer/src/Utils/JsonManipulator.php (3)
JsonManipulator(10-81)fromFile(16-41)getProperty(52-79).vortex/installer/src/Prompts/Handlers/AbstractHandler.php (3)
AbstractHandler(10-233)isInstalled(178-180)getResponseAsArray(208-214)
🪛 GitHub Actions: Vortex - Test installer
.vortex/installer/src/Prompts/Handlers/Modules.php
[error] 84-84: There should be no white space after an opening "(" (Drupal.WhiteSpace.OpenBracketSpacing.OpeningWhitespace).
[error] 100-100: There should be no white space after an opening "(" (Drupal.WhiteSpace.OpenBracketSpacing.OpeningWhitespace).
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: build (1)
- GitHub Check: build (0)
- GitHub Check: vortex-test-workflow (4)
- GitHub Check: vortex-test-common
- GitHub Check: vortex-test-workflow (0)
- GitHub Check: vortex-test-workflow (3)
- GitHub Check: vortex-test-workflow (2)
- GitHub Check: vortex-test-workflow (1)
- GitHub Check: vortex-test-docs
🔇 Additional comments (13)
tests/phpunit/Drupal/SwitchableSettingsTest.php (1)
81-81: PHPCS ignore directives with custom markers are non-functional and should use actual rule names instead.The markers (
#;< MODULE_CONFIG_SPLIT, etc.) are Vortex template system tokens designed to be removed during project scaffolding, not PHPCS rule identifiers. PHPCS cannot suppress violations using these custom markers because they don't correspond to any defined rules in your PHPCS configuration.Replace
// phpcs:ignore #;< MODULE_*with:
- Actual PHPCS rule names (e.g.,
// phpcs:ignore Drupal.Commenting.DocComment.MissingShort), OR- Generic
// phpcs:ignore Generic.Commentingif suppressing all rules around the blockThe same issue exists in pre-existing SERVICE_CLAMAV and SERVICE_REDIS markers (lines 22, 80, 239, 314).
Also applies to: 170-171, 238-238, 315-315, 549-550, 679-680
⛔ Skipped due to learnings
Learnt from: AlexSkrypnyk Repo: drevops/vortex PR: 1693 File: .vortex/installer/tests/Fixtures/install/deploy_type_none_gha/.github/workflows/build-test-deploy.yml:1-4 Timestamp: 2025-06-01T08:08:59.071Z Learning: Test fixture files (typically located in directories containing "fixture" or "test") should not be reviewed for code quality issues as they may intentionally contain incomplete, invalid, or malformed data for testing purposes..vortex/installer/src/Prompts/Handlers/ModulePrefix.php (1)
16-16: LGTM!The label clarification distinguishes this prompt from the new Modules prompt, improving user experience.
.env (1)
65-75: LGTM!The MODULE_SHIELD markers provide structural boundaries for conditional content management, aligning with the new Modules prompt functionality.
docker-compose.yml (1)
60-64: LGTM!The MODULE_SHIELD delimiters enable conditional inclusion of Shield-related environment variables, consistent with the module selection feature.
scripts/custom/provision-10-example.sh (1)
40-43: LGTM!The MODULES section markers enable targeted removal of module installation commands when modules are deselected, aligning with the new Modules prompt handler's cleanup logic.
.vortex/installer/tests/Unit/Handlers/AbstractPromptManagerTestCase.php (1)
22-22: LGTM!The test infrastructure correctly integrates the new Modules prompt with appropriate defaults for both new and pre-installed project scenarios.
Also applies to: 128-128, 198-198
.vortex/installer/src/Utils/JsonManipulator.php (1)
32-40: LGTM!The guarded construction provides robust error handling for invalid JSON, supporting graceful fallback in module discovery when composer.json is malformed.
.vortex/installer/src/Prompts/PromptManager.php (3)
68-68: LGTM!The total response count correctly reflects the addition of the Modules prompt.
139-139: LGTM!The Modules prompt is correctly integrated into the flow:
- Multi-select prompt in the Drupal section
- Processor execution in the appropriate sequence
- Response summary display
Also applies to: 282-282, 374-374
550-553: Condition change is safe—no handlers return empty arrays.The review concern about the condition changing from
!empty($options)tois_array($options)is not valid in practice. All handlers that return arrays either:
- Return hardcoded non-empty arrays (Starter, Theme, Profile, Services, HostingProvider, etc.)
- Conditionally unset items but preserve non-empty arrays:
- DeployTypes: starts 4 items, removes max 1 → minimum 3
- DatabaseDownloadSource: starts 6 items, removes max 1 → minimum 5
- CiProvider: starts 3 items, removes max 1 → minimum 2
- Return NULL (AbstractHandler default)
No handler can return an empty array
[], so the semantic difference between checking!empty()andis_array()never manifests. The scroll behavior is only applied to non-empty options, and existing prompts are not affected..vortex/installer/composer.json (2)
21-21: Dependency bump is compatible; no breaking changes detected.The changelog for v0.14.0 shows only additive features (regex support for
removeLine()andfindMatchingPath()methods) and internal improvements. No breaking changes are documented, and the updates include coding standards improvements and performance enhancements. The semantic versioning bump (0.13.1 → 0.14.0) is consistent with backward-compatible changes. Safe to upgrade.
91-94: Removal of --clear-cache is safe.Rector handles caching automatically and --clear-cache is optional, used only when explicitly forcing cache rebuild after config/rule changes. The changes to your lint and lint-fix scripts are correct and align with Rector 2.2.8's caching behavior.
.vortex/installer/tests/Functional/Handlers/ModulesInstallTest.php (1)
17-146: Functional coverage and Env wiring for Modules look solidThe functional scenarios cleanly drive the installer via
PromptManager::makeEnvName(Modules::id())andConverter::toList(), and the post‑install assertions (assertSutNotContains) exercise both single‑module and “none selected” paths in a realistic way. This gives good end‑to‑end confidence in the new handler’s behavior.
4bd3097 to
49739f2
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #2108 +/- ##
===========================================
+ Coverage 70.02% 70.35% +0.33%
===========================================
Files 97 98 +1
Lines 4898 4973 +75
Branches 44 44
===========================================
+ Hits 3430 3499 +69
- Misses 1468 1474 +6 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
49739f2 to
d57a05c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
♻️ Duplicate comments (1)
.vortex/installer/tests/Unit/Handlers/ModulesPromptManagerTest.php (1)
102-109: Test doesn't exercise the invalid JSON discovery path as intended.The test scenario "modules - discovery - invalid composer.json" doesn't actually invoke the discovery logic because
isInstalled()returnsFALSE(nostubVortexProject()call), soModules::discover()returnsNULLbefore ever attempting to readcomposer.json.To properly test the invalid JSON fallback, the callback should mark the project as installed:
'modules - discovery - invalid composer.json' => [ [], - $expected_defaults, + $expected_installed, function (AbstractPromptManagerTestCase $test, Config $config): void { + $test->stubVortexProject($config); // Invalid JSON causes discovery to fail and fall back to defaults. File::dump(static::$sut . '/composer.json', 'invalid json content'); }, ],Note: Also corrected the expected result to
$expected_installedsince a Vortex project should use installed defaults.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (71)
.vortex/docs/static/img/installer.svgis excluded by!**/*.svg.vortex/installer/composer.lockis excluded by!**/*.lock.vortex/installer/tests/Fixtures/install/_baseline/scripts/custom/provision-10-example.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_admin_toolbar/composer.jsonis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_admin_toolbar/scripts/custom/provision-10-example.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_coffee/composer.jsonis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_coffee/scripts/custom/provision-10-example.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_config_split/composer.jsonis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_config_split/scripts/custom/provision-10-example.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_config_split/tests/phpunit/Drupal/EnvironmentSettingsTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_config_split/tests/phpunit/Drupal/SwitchableSettingsTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_config_split/web/sites/default/includes/modules/-settings.config_split.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_config_update/composer.jsonis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_config_update/scripts/custom/provision-10-example.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_environment_indicator/composer.jsonis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_environment_indicator/scripts/custom/provision-10-example.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_environment_indicator/tests/phpunit/Drupal/EnvironmentSettingsTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_environment_indicator/tests/phpunit/Drupal/SwitchableSettingsTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_environment_indicator/web/sites/default/includes/modules/-settings.environment_indicator.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_pathauto/composer.jsonis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_pathauto/scripts/custom/provision-10-example.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_redirect/composer.jsonis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_redirect/scripts/custom/provision-10-example.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_robotstxt/composer.jsonis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_robotstxt/scripts/custom/provision-10-example.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_robotstxt/tests/behat/features/-robotstxt.featureis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_robotstxt/tests/phpunit/Drupal/EnvironmentSettingsTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_robotstxt/web/sites/default/includes/modules/-settings.robotstxt.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_seckit/composer.jsonis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_seckit/tests/phpunit/Drupal/EnvironmentSettingsTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_seckit/web/sites/default/includes/modules/-settings.seckit.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_seckit_shield_stage_file_proxy/.envis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_seckit_shield_stage_file_proxy/composer.jsonis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_seckit_shield_stage_file_proxy/docker-compose.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_seckit_shield_stage_file_proxy/scripts/custom/provision-10-example.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_seckit_shield_stage_file_proxy/tests/phpunit/Drupal/EnvironmentSettingsTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_seckit_shield_stage_file_proxy/tests/phpunit/Drupal/SwitchableSettingsTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_seckit_shield_stage_file_proxy/web/sites/default/includes/modules/-settings.seckit.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_seckit_shield_stage_file_proxy/web/sites/default/includes/modules/-settings.shield.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_seckit_shield_stage_file_proxy/web/sites/default/includes/modules/-settings.stage_file_proxy.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_shield/.envis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_shield/composer.jsonis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_shield/docker-compose.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_shield/scripts/custom/provision-10-example.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_shield/tests/phpunit/Drupal/EnvironmentSettingsTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_shield/tests/phpunit/Drupal/SwitchableSettingsTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_shield/web/sites/default/includes/modules/-settings.shield.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_stage_file_proxy/composer.jsonis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_stage_file_proxy/scripts/custom/provision-10-example.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_stage_file_proxy/tests/phpunit/Drupal/SwitchableSettingsTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_stage_file_proxy/web/sites/default/includes/modules/-settings.stage_file_proxy.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_none/.envis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_none/composer.jsonis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_none/docker-compose.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_none/scripts/custom/provision-10-example.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_none/tests/behat/features/-robotstxt.featureis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_none/tests/phpunit/Drupal/EnvironmentSettingsTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_none/tests/phpunit/Drupal/SwitchableSettingsTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_none/web/sites/default/includes/modules/-settings.config_split.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_none/web/sites/default/includes/modules/-settings.environment_indicator.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_none/web/sites/default/includes/modules/-settings.robotstxt.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_none/web/sites/default/includes/modules/-settings.seckit.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_none/web/sites/default/includes/modules/-settings.shield.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_none/web/sites/default/includes/modules/-settings.stage_file_proxy.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/names/scripts/custom/provision-10-example.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/non_interactive_config_file/scripts/custom/provision-10-example.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/non_interactive_config_string/scripts/custom/provision-10-example.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/services_no_clamav/scripts/custom/provision-10-example.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/services_no_redis/scripts/custom/provision-10-example.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/services_no_solr/scripts/custom/provision-10-example.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/services_none/scripts/custom/provision-10-example.shis excluded by!.vortex/installer/tests/Fixtures/**
📒 Files selected for processing (12)
.env(1 hunks).vortex/installer/composer.json(2 hunks).vortex/installer/src/Prompts/Handlers/ModulePrefix.php(1 hunks).vortex/installer/src/Prompts/Handlers/Modules.php(1 hunks).vortex/installer/src/Prompts/PromptManager.php(6 hunks).vortex/installer/src/Utils/JsonManipulator.php(1 hunks).vortex/installer/tests/Functional/Handlers/ModulesInstallTest.php(1 hunks).vortex/installer/tests/Unit/Handlers/AbstractPromptManagerTestCase.php(3 hunks).vortex/installer/tests/Unit/Handlers/ModulesPromptManagerTest.php(1 hunks)docker-compose.yml(1 hunks)scripts/custom/provision-10-example.sh(1 hunks)tests/phpunit/Drupal/SwitchableSettingsTest.php(6 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-06-01T08:10:15.903Z
Learnt from: AlexSkrypnyk
Repo: drevops/vortex PR: 1693
File: .vortex/installer/src/Prompts/Handlers/Internal.php:52-52
Timestamp: 2025-06-01T08:10:15.903Z
Learning: The File utility class in .vortex/installer/src/Utils/File.php extends AlexSkrypnyk\File\File, and methods like collapseRepeatedEmptyLines() are available from this external parent class dependency.
Applied to files:
.vortex/installer/composer.json.vortex/installer/src/Utils/JsonManipulator.php
📚 Learning: 2025-09-13T04:14:41.765Z
Learnt from: AlexSkrypnyk
Repo: drevops/vortex PR: 2011
File: .vortex/tests/phpunit/Traits/Steps/StepAhoyTrait.php:20-23
Timestamp: 2025-09-13T04:14:41.765Z
Learning: In .vortex/tests/phpunit PHPUnit tests using cmd/cmdFail helpers, patterns in the expected output arrays support '*' markers for positive assertions (text must be present) and '!' markers for negative assertions (text must NOT be present). The '*' markers are NOT literal text matches but rather indicate expected presence.
Applied to files:
.vortex/installer/tests/Unit/Handlers/ModulesPromptManagerTest.php
🧬 Code graph analysis (6)
.vortex/installer/src/Prompts/PromptManager.php (3)
.vortex/installer/src/Prompts/Handlers/Modules.php (1)
Modules(10-178).vortex/installer/src/Prompts/Handlers/HandlerInterface.php (2)
id(19-19)options(76-76).vortex/installer/src/Prompts/Handlers/AbstractHandler.php (2)
id(52-62)options(95-97)
.vortex/installer/src/Prompts/Handlers/Modules.php (3)
.vortex/installer/src/Utils/File.php (2)
File(10-121)removeTokenAsync(93-100).vortex/installer/src/Utils/JsonManipulator.php (3)
JsonManipulator(10-81)fromFile(16-41)getProperty(52-79).vortex/installer/src/Prompts/Handlers/AbstractHandler.php (3)
AbstractHandler(10-233)isInstalled(178-180)getResponseAsArray(208-214)
.vortex/installer/tests/Functional/Handlers/ModulesInstallTest.php (5)
.vortex/installer/src/Prompts/Handlers/Modules.php (2)
Modules(10-178)getAvailableModules(123-137).vortex/installer/src/Prompts/PromptManager.php (2)
PromptManager(61-625)makeEnvName(443-445).vortex/installer/src/Utils/Converter.php (1)
Converter(9-17).vortex/installer/src/Utils/Env.php (2)
Env(7-241)put(24-26).vortex/installer/tests/Functional/FunctionalTestCase.php (1)
assertSutNotContains(130-145)
scripts/custom/provision-10-example.sh (1)
scripts/vortex/provision.sh (2)
task(71-71)drush(77-77)
.vortex/installer/tests/Unit/Handlers/ModulesPromptManagerTest.php (4)
.vortex/installer/src/Prompts/Handlers/Modules.php (2)
Modules(10-178)getAvailableModules(123-137).vortex/installer/src/Utils/Config.php (1)
Config(14-126).vortex/installer/src/Utils/File.php (1)
File(10-121).vortex/installer/tests/Unit/Handlers/AbstractPromptManagerTestCase.php (6)
AbstractPromptManagerTestCase(50-276)dataProviderRunPrompts(115-115)getExpectedDefaults(120-151)getExpectedInstalled(156-166)stubVortexProject(262-268)stubComposerJsonDependencies(230-252)
.vortex/installer/tests/Unit/Handlers/AbstractPromptManagerTestCase.php (2)
.vortex/installer/src/Prompts/Handlers/Modules.php (2)
Modules(10-178)getAvailableModules(123-137).vortex/installer/src/Prompts/Handlers/AbstractHandler.php (1)
id(52-62)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
- GitHub Check: build (0)
- GitHub Check: build (1)
- GitHub Check: vortex-test-workflow (4)
- GitHub Check: vortex-test-workflow (2)
- GitHub Check: vortex-test-workflow (3)
- GitHub Check: vortex-test-workflow (0)
- GitHub Check: vortex-test-common
- GitHub Check: vortex-test-workflow (1)
- GitHub Check: vortex-test-installer (8.4)
- GitHub Check: vortex-test-installer (8.3)
- GitHub Check: vortex-test-docs
🔇 Additional comments (21)
scripts/custom/provision-10-example.sh (2)
40-40: Marker format correctly aligns with provisioning system conventions.The
#;< MODULESand#;> MODULESdelimiters follow the established pattern for section markers in this file (e.g.,#;< SERVICE_REDIS,#;> SERVICE_REDIS) and enable the provisioning system to identify and manage the MODULES section during the installer workflow. Placement is correct—the markers properly encapsulate the drush module installation command.Also applies to: 43-43
40-43: Markers are decorative documentation, not parsed by the provisioning system.The provisioning system executes custom scripts via direct invocation—
"${file}"in a loop—without parsing section markers. The mainprovision.shscript contains no logic to recognize, parse, or conditionally execute sections based on#;< MODULESmarkers or similar delimiters. The markers inprovision-10-example.share organizational comments for developers, not consumed by any provisioning or installer system. No registration or whitelisting mechanism exists.Likely an incorrect or invalid review comment.
tests/phpunit/Drupal/SwitchableSettingsTest.php (2)
170-171: Consistent marker pattern applied across all module test blocks.The phpcs:ignore marker pattern is consistently applied to MODULE_CONFIG_SPLIT, MODULE_ENVIRONMENT_INDICATOR, MODULE_SHIELD, and MODULE_STAGE_FILE_PROXY test blocks, matching the existing pattern for SERVICE_* markers.
Also applies to: 238-238, 315-315, 549-550, 679-679
81-81: No changes required—marker syntax is intentional Vortex framework pattern.The
// phpcs:ignore #;< MODULE_*markers are a documented part of the Vortex project template. Comments starting with '#;', '#;<' or '#;>' are explaining Vortex inner workings and can be safely removed for your project. They are automatically removed when the installer runs. The markers are used consistently throughout the codebase (docker-compose.yml, .ahoy.yml, .lagoon.yml, and test files) for feature flag management, and even appear in phpcs.xml itself. The// phpcs:ignoreprefix suppresses PHPCS warnings for the marker line itself, while the markers are processed by the Vortex installer. The code changes follow the established pattern correctly.docker-compose.yml (1)
60-64: LGTM! Shield credentials correctly wrapped with MODULE_SHIELD markers.The MODULE_SHIELD markers appropriately wrap the Shield-specific environment variables (DRUPAL_SHIELD_USER and DRUPAL_SHIELD_PASS) and their associated comment. This pattern is consistent with existing SERVICE_* markers throughout the file (e.g., SERVICE_REDIS, SERVICE_SOLR) and enables conditional inclusion of Shield configuration based on module selection.
.vortex/installer/composer.json (2)
21-21: LGTM! Dependency version update looks appropriate.The bump to alexskrypnyk/file ^0.14 aligns with the need for newer file utility features used in the Modules handler.
91-91: LGTM! Rector script simplification improves execution time.Removing the
--clear-cacheflag from both lint commands is a reasonable optimization that speeds up the linting process without compromising the checks.Also applies to: 94-94
.vortex/installer/tests/Unit/Handlers/AbstractPromptManagerTestCase.php (1)
22-22: LGTM! Modules handler properly integrated into test scaffolding.The addition of the Modules import, expected defaults, and TUI answers follows the established pattern for other handlers and ensures comprehensive test coverage.
Also applies to: 129-129, 199-199
.vortex/installer/src/Prompts/Handlers/ModulePrefix.php (1)
16-16: LGTM! Label clarification improves UX.The updated label "Custom modules prefix" better distinguishes this prompt from the new Modules prompt (for contributed modules), reducing potential user confusion.
.vortex/installer/src/Prompts/PromptManager.php (2)
25-25: LGTM! Modules handler properly integrated into prompt flow.The changes correctly:
- Import the Modules handler
- Increment TOTAL_RESPONSES to account for the new prompt
- Add the Modules multiselect prompt in the appropriate position
- Include Modules processing in the correct sequence
- Display Modules in the response summary
Also applies to: 68-68, 139-139, 282-282, 374-374
550-553: LGTM! Options handling now supports empty arrays.Removing the
!empty($options)check allows multiselect prompts (like Modules) to accept empty arrays, which is necessary when users deselect all options. This is the correct behavior for the Modules feature..vortex/installer/tests/Functional/Handlers/ModulesInstallTest.php (2)
17-131: LGTM! Comprehensive functional test coverage for module exclusions.The test data provider covers:
- Individual module exclusions (all 11 available modules)
- Composite exclusion (multiple modules at once)
- Edge case: no modules selected
The implementation is clean, consistent, and properly uses environment variables to configure the installer behavior.
142-146: LGTM! Clean and reusable helper method.The
getModulesExcept()helper method provides a clear, type-safe way to compute the expected module list for test scenarios..vortex/installer/src/Prompts/Handlers/Modules.php (8)
15-31: LGTM! UI methods properly implemented.The label, hint, and options methods follow the expected pattern and provide clear user guidance for the multiselect prompt.
36-39: LGTM! Sensible default behavior.Defaulting to all modules selected (none removed) follows the principle of least surprise and ensures existing projects aren't inadvertently broken during installation.
44-63: LGTM! Discovery logic is robust and well-structured.The method correctly:
- Checks installation state before discovery
- Handles errors gracefully with NULL returns
- Filters discovered modules to the available list
- Sorts for consistent ordering
86-96: LGTM! Provision script cleanup handles multi-module commands correctly.The nested callback properly:
- Preserves indentation
- Removes only the specified module from multi-module drush commands
- Collapses extra spaces
- Uses NULL coalescing to handle regex failures safely
105-112: LGTM! Token removal logic is correct.The handler properly removes both module-specific tokens (
MODULE_<NAME>) and the generalMODULEtoken (when no modules are selected), enabling template conditional content.
123-137: LGTM! Clear module inventory.The static method provides a well-defined list of 11 contributed modules, serving as the single source of truth for the installer's module selection feature.
148-176: LGTM! Composer.json parsing is robust and correct.The method properly:
- Checks file existence before reading
- Guards against NULL from JsonManipulator (past issue resolved)
- Validates the require property is an array
- Filters to
drupal/*packages while excludingdrupal/core-*- Extracts module names correctly
79-80: Verify composer.json cleanup doesn't leave formatting issues.The regex pattern
/\s*"drupal\/...is used to remove package entries from composer.json and replaces with a single\n. While the pattern appears structurally sound, I confirmed thatcollapseRepeatedEmptyLines()from the parent File utility is available but NOT used anywhere in the codebase. This means no post-processing consolidates multiple consecutive newlines.However, since
replaceContentInFile()is inherited from the external AlexSkrypnyk\File\File library and its implementation is inaccessible in this environment, I cannot definitively verify whether:
- The method itself handles blank line consolidation internally, or
- The pattern actually leaves multiple consecutive blank lines in practice
Given this uncertainty and that the same pattern is consistently used across multiple locations (Services.php, Modules.php), recommend manual verification that the generated composer.json has proper formatting with no excessive blank lines in actual installation scenarios.
d57a05c to
11ee7f5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
♻️ Duplicate comments (1)
.vortex/installer/src/Utils/JsonManipulator.php (1)
32-38: Consider catching\Throwablefor broader error coverage.The current catch block only catches
\Exception, which may missErrorsubclasses (likeTypeErrororParseError) that could be thrown during JSON parsing in the parent constructor.Apply this diff for more robust error handling:
try { $instance = new self($contents); } - catch (\Exception) { + catch (\Throwable) { // Invalid JSON. return NULL; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (72)
.vortex/docs/static/img/installer.svgis excluded by!**/*.svg.vortex/installer/composer.lockis excluded by!**/*.lock.vortex/installer/tests/Fixtures/install/_baseline/scripts/custom/provision-10-example.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_admin_toolbar/composer.jsonis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_admin_toolbar/scripts/custom/provision-10-example.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_coffee/composer.jsonis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_coffee/scripts/custom/provision-10-example.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_config_split/composer.jsonis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_config_split/scripts/custom/provision-10-example.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_config_split/tests/phpunit/Drupal/EnvironmentSettingsTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_config_split/tests/phpunit/Drupal/SwitchableSettingsTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_config_split/web/sites/default/includes/modules/-settings.config_split.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_config_update/composer.jsonis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_config_update/scripts/custom/provision-10-example.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_environment_indicator/composer.jsonis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_environment_indicator/scripts/custom/provision-10-example.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_environment_indicator/tests/phpunit/Drupal/EnvironmentSettingsTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_environment_indicator/tests/phpunit/Drupal/SwitchableSettingsTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_environment_indicator/web/sites/default/includes/modules/-settings.environment_indicator.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_pathauto/composer.jsonis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_pathauto/scripts/custom/provision-10-example.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_redirect/composer.jsonis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_redirect/scripts/custom/provision-10-example.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_robotstxt/composer.jsonis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_robotstxt/scripts/custom/provision-10-example.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_robotstxt/tests/behat/features/-robotstxt.featureis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_robotstxt/tests/phpunit/Drupal/EnvironmentSettingsTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_robotstxt/web/sites/default/includes/modules/-settings.robotstxt.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_seckit/composer.jsonis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_seckit/tests/phpunit/Drupal/EnvironmentSettingsTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_seckit/web/sites/default/includes/modules/-settings.seckit.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_seckit_shield_stage_file_proxy/.envis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_seckit_shield_stage_file_proxy/composer.jsonis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_seckit_shield_stage_file_proxy/docker-compose.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_seckit_shield_stage_file_proxy/scripts/custom/provision-10-example.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_seckit_shield_stage_file_proxy/tests/phpunit/Drupal/EnvironmentSettingsTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_seckit_shield_stage_file_proxy/tests/phpunit/Drupal/SwitchableSettingsTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_seckit_shield_stage_file_proxy/web/sites/default/includes/modules/-settings.seckit.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_seckit_shield_stage_file_proxy/web/sites/default/includes/modules/-settings.shield.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_seckit_shield_stage_file_proxy/web/sites/default/includes/modules/-settings.stage_file_proxy.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_shield/.envis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_shield/composer.jsonis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_shield/docker-compose.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_shield/scripts/custom/provision-10-example.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_shield/tests/phpunit/Drupal/EnvironmentSettingsTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_shield/tests/phpunit/Drupal/SwitchableSettingsTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_shield/web/sites/default/includes/modules/-settings.shield.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_stage_file_proxy/.envis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_stage_file_proxy/composer.jsonis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_stage_file_proxy/scripts/custom/provision-10-example.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_stage_file_proxy/tests/phpunit/Drupal/SwitchableSettingsTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_no_stage_file_proxy/web/sites/default/includes/modules/-settings.stage_file_proxy.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_none/.envis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_none/composer.jsonis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_none/docker-compose.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_none/scripts/custom/provision-10-example.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_none/tests/behat/features/-robotstxt.featureis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_none/tests/phpunit/Drupal/EnvironmentSettingsTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_none/tests/phpunit/Drupal/SwitchableSettingsTest.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_none/web/sites/default/includes/modules/-settings.config_split.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_none/web/sites/default/includes/modules/-settings.environment_indicator.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_none/web/sites/default/includes/modules/-settings.robotstxt.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_none/web/sites/default/includes/modules/-settings.seckit.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_none/web/sites/default/includes/modules/-settings.shield.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/modules_none/web/sites/default/includes/modules/-settings.stage_file_proxy.phpis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/names/scripts/custom/provision-10-example.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/non_interactive_config_file/scripts/custom/provision-10-example.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/non_interactive_config_string/scripts/custom/provision-10-example.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/services_no_clamav/scripts/custom/provision-10-example.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/services_no_redis/scripts/custom/provision-10-example.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/services_no_solr/scripts/custom/provision-10-example.shis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/install/services_none/scripts/custom/provision-10-example.shis excluded by!.vortex/installer/tests/Fixtures/**
📒 Files selected for processing (12)
.env(1 hunks).vortex/installer/composer.json(2 hunks).vortex/installer/src/Prompts/Handlers/ModulePrefix.php(1 hunks).vortex/installer/src/Prompts/Handlers/Modules.php(1 hunks).vortex/installer/src/Prompts/PromptManager.php(6 hunks).vortex/installer/src/Utils/JsonManipulator.php(1 hunks).vortex/installer/tests/Functional/Handlers/ModulesInstallTest.php(1 hunks).vortex/installer/tests/Unit/Handlers/AbstractPromptManagerTestCase.php(3 hunks).vortex/installer/tests/Unit/Handlers/ModulesPromptManagerTest.php(1 hunks)docker-compose.yml(1 hunks)scripts/custom/provision-10-example.sh(1 hunks)tests/phpunit/Drupal/SwitchableSettingsTest.php(6 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-09-13T04:14:41.765Z
Learnt from: AlexSkrypnyk
Repo: drevops/vortex PR: 2011
File: .vortex/tests/phpunit/Traits/Steps/StepAhoyTrait.php:20-23
Timestamp: 2025-09-13T04:14:41.765Z
Learning: In .vortex/tests/phpunit PHPUnit tests using cmd/cmdFail helpers, patterns in the expected output arrays support '*' markers for positive assertions (text must be present) and '!' markers for negative assertions (text must NOT be present). The '*' markers are NOT literal text matches but rather indicate expected presence.
Applied to files:
.vortex/installer/tests/Unit/Handlers/ModulesPromptManagerTest.php
📚 Learning: 2025-06-01T08:10:15.903Z
Learnt from: AlexSkrypnyk
Repo: drevops/vortex PR: 1693
File: .vortex/installer/src/Prompts/Handlers/Internal.php:52-52
Timestamp: 2025-06-01T08:10:15.903Z
Learning: The File utility class in .vortex/installer/src/Utils/File.php extends AlexSkrypnyk\File\File, and methods like collapseRepeatedEmptyLines() are available from this external parent class dependency.
Applied to files:
.vortex/installer/src/Utils/JsonManipulator.php.vortex/installer/composer.json
🧬 Code graph analysis (6)
.vortex/installer/tests/Unit/Handlers/ModulesPromptManagerTest.php (5)
.vortex/installer/src/Prompts/Handlers/Modules.php (2)
Modules(10-178)getAvailableModules(123-137).vortex/installer/src/Utils/Config.php (1)
Config(14-126).vortex/installer/src/Utils/File.php (1)
File(10-121).vortex/installer/tests/Functional/Handlers/ModulesInstallTest.php (1)
CoversClass(14-148).vortex/installer/tests/Unit/Handlers/AbstractPromptManagerTestCase.php (6)
AbstractPromptManagerTestCase(50-276)dataProviderRunPrompts(115-115)getExpectedDefaults(120-151)getExpectedInstalled(156-166)stubVortexProject(262-268)stubComposerJsonDependencies(230-252)
.vortex/installer/tests/Functional/Handlers/ModulesInstallTest.php (5)
.vortex/installer/src/Prompts/Handlers/Modules.php (2)
Modules(10-178)getAvailableModules(123-137).vortex/installer/src/Prompts/PromptManager.php (2)
PromptManager(61-625)makeEnvName(443-445).vortex/installer/src/Utils/Converter.php (1)
Converter(9-17).vortex/installer/src/Utils/Env.php (2)
Env(7-241)put(24-26).vortex/installer/tests/Functional/FunctionalTestCase.php (1)
assertSutNotContains(130-145)
.vortex/installer/tests/Unit/Handlers/AbstractPromptManagerTestCase.php (3)
.vortex/installer/src/Prompts/Handlers/Modules.php (2)
Modules(10-178)getAvailableModules(123-137).vortex/installer/src/Prompts/Handlers/AbstractHandler.php (1)
id(52-62).vortex/installer/src/Prompts/Handlers/HandlerInterface.php (1)
id(19-19)
.vortex/installer/src/Prompts/Handlers/Modules.php (4)
.vortex/installer/src/Utils/File.php (2)
File(10-121)removeTokenAsync(93-100).vortex/installer/src/Utils/JsonManipulator.php (3)
JsonManipulator(10-81)fromFile(16-41)getProperty(52-79).vortex/installer/src/Prompts/Handlers/AbstractHandler.php (3)
AbstractHandler(10-233)isInstalled(178-180)getResponseAsArray(208-214).vortex/installer/src/Prompts/Handlers/ModulePrefix.php (3)
hint(22-24)default(43-49)process(92-115)
.vortex/installer/src/Prompts/PromptManager.php (3)
.vortex/installer/src/Prompts/Handlers/Modules.php (1)
Modules(10-178).vortex/installer/src/Prompts/Handlers/AbstractHandler.php (2)
id(52-62)options(95-97).vortex/installer/src/Prompts/Handlers/HandlerInterface.php (2)
id(19-19)options(76-76)
scripts/custom/provision-10-example.sh (1)
scripts/vortex/provision.sh (2)
task(71-71)drush(77-77)
🪛 dotenv-linter (4.0.0)
.env
[warning] 64-64: [ExtraBlankLine] Extra blank line detected
(ExtraBlankLine)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
- GitHub Check: build (1)
- GitHub Check: build (0)
- GitHub Check: vortex-test-workflow (3)
- GitHub Check: vortex-test-workflow (4)
- GitHub Check: vortex-test-workflow (0)
- GitHub Check: vortex-test-workflow (1)
- GitHub Check: vortex-test-workflow (2)
- GitHub Check: vortex-test-installer (8.4)
- GitHub Check: vortex-test-common
- GitHub Check: vortex-test-installer (8.3)
- GitHub Check: vortex-test-docs
🔇 Additional comments (10)
tests/phpunit/Drupal/SwitchableSettingsTest.php (1)
81-81: LGTM! Marker annotations are correctly positioned.The phpcs:ignore annotations around MODULE_* marker blocks are appropriate. These suppress linting warnings for the special marker syntax used to delineate conditional module configuration blocks throughout the codebase.
Also applies to: 170-171, 238-238, 315-315, 549-550, 679-680
scripts/custom/provision-10-example.sh (1)
40-43: LGTM! Module markers correctly positioned.The MODULES markers appropriately wrap the contrib module installation command, enabling the installer to remove unselected modules from this provisioning script.
.vortex/installer/src/Prompts/Handlers/ModulePrefix.php (1)
16-16: LGTM! Clearer label for user experience.The addition of "Custom" to the label clarifies that this prefix applies specifically to custom modules, not contributed modules.
.vortex/installer/tests/Unit/Handlers/AbstractPromptManagerTestCase.php (1)
22-22: LGTM! Modules handler properly integrated into test infrastructure.The Modules handler is correctly added to:
- Imports (line 22)
- Default expectations with all modules selected (line 129)
- TUI default answers (line 199)
This aligns with the PR objective of defaulting to all modules selected for new installations.
Also applies to: 129-129, 199-199
.env (1)
65-78: LGTM! Module-specific configuration correctly wrapped with markers.The MODULE_STAGE_FILE_PROXY and MODULE_SHIELD markers appropriately delineate module-specific environment variables, enabling the installer to remove these configuration blocks when the respective modules are not selected.
docker-compose.yml (1)
60-64: LGTM! Shield credentials correctly wrapped with MODULE_SHIELD markers.The MODULE_SHIELD markers appropriately wrap only the Shield-specific environment variables (DRUPAL_SHIELD_USER and DRUPAL_SHIELD_PASS), while correctly leaving DRUPAL_CONFIG_PATH outside the markers as it's not module-specific.
.vortex/installer/src/Prompts/PromptManager.php (2)
25-25: Modules prompt is cleanly integrated into the prompt flow and processingThe new Modules handler is wired consistently:
- Imported and reflected in
TOTAL_RESPONSES.- Added as a
multiselectin the “Drupal” section, aligned with how Services/Tools are handled.- Included in
runProcessors()in a sensible place (after CodeProvider, before Starter), and- Surfaced in
getResponsesSummary()viaConverter::toList(...).This aligns well with the Modules handler contract and the expectations in the new unit/functional tests.
Also applies to: 68-68, 120-150, 259-283, 362-377
549-553: Behavior change: options now passed even for empty arrays
args()now applies:$options = $handler->options($responses); if (is_array($options)) { $args['options'] = $options; $args['scroll'] = 10; }Previously, handlers returning an empty array from
options()would not set anoptionskey at all; now they’ll produce prompts with an explicit empty options list. That’s usually more correct, but it can change behavior if any existing handler used[]as a sentinel to “disable” a select/multiselect prompt.Please double‑check existing handlers for that pattern to ensure this doesn’t accidentally surface prompts with no choices or regress any edge cases.
.vortex/installer/tests/Functional/Handlers/ModulesInstallTest.php (1)
17-146: Functional coverage for Modules removal looks solidThe data provider thoroughly exercises the Modules flow:
- Each individual module can be deselected and is asserted absent from the SUT.
- Combined and “none” scenarios validate that composer entries (and related artifacts) for deselected modules are removed.
- Env wiring via
PromptManager::makeEnvName(Modules::id())andConverter::toList()matches the prompt/config conventions elsewhere.This gives good end‑to‑end confidence in the new Modules behavior.
.vortex/installer/src/Prompts/Handlers/Modules.php (1)
10-63: Modules handler behavior matches discovery + defaults contractThe handler cleanly implements the intended behavior:
- Defaults to all available modules selected for new installs.
- For existing Vortex projects,
discover()readscomposer.json, extractsdrupal/*(excludingdrupal/core-*), intersects withgetAvailableModules(), and returns a sorted list.getModulesFromComposerFile()safely handles missing/unreadable/invalid composer.json viaJsonManipulator::fromFile()+instanceofguard and returnsNULLon error, which feeds the “fallback to defaults” path exercised in tests.This is consistent with the unit tests and the installer semantics.
Also applies to: 148-176
| # Drupal maintenance theme name. | ||
| DRUPAL_MAINTENANCE_THEME=your_site_theme | ||
|
|
||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick | 🔵 Trivial
Consider removing the extra blank line.
Static analysis detected an extra blank line at line 64. While not functionally problematic, removing it would improve consistency with the rest of the file.
Apply this diff:
DRUPAL_MAINTENANCE_THEME=your_site_theme
-
#;< MODULE_STAGE_FILE_PROXY📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| DRUPAL_MAINTENANCE_THEME=your_site_theme | |
| #;< MODULE_STAGE_FILE_PROXY |
🧰 Tools
🪛 dotenv-linter (4.0.0)
[warning] 64-64: [ExtraBlankLine] Extra blank line detected
(ExtraBlankLine)
🤖 Prompt for AI Agents
In .env around line 64, there is an extra blank line that breaks visual
consistency; remove the empty line so there is no blank line at line 64,
ensuring the file has contiguous entries and matches the surrounding formatting.
| "phpcs", | ||
| "phpstan", | ||
| "rector --clear-cache --dry-run" | ||
| "rector --dry-run" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Verify removal of --clear-cache flags from Rector commands.
The --clear-cache flag has been removed from both the lint and lint-fix scripts for Rector. Ensure this change is intentional and that Rector's caching behavior in the current version (^2.2.8) doesn't require explicit cache clearing for consistent results.
Run the following to verify Rector behavior without the flag:
Also applies to: 94-94
🏁 Script executed:
#!/bin/bash
# Description: Test Rector lint commands to ensure they work correctly without --clear-cache
# Check if rector runs successfully without --clear-cache
cd .vortex/installer
composer install --quiet
echo "=== Testing rector --dry-run (lint) ==="
./vendor/bin/rector --dry-run || echo "Rector dry-run failed"
echo ""
echo "=== Check Rector documentation for caching behavior ==="
./vendor/bin/rector --help | grep -i cacheLength of output: 341
🏁 Script executed:
#!/bin/bash
# Examine the composer.json file to see the rector version and changes
echo "=== Checking rector version in composer.json ==="
cat .vortex/installer/composer.json | grep -A 2 -B 2 '"rector'
echo ""
echo "=== Viewing lines 85-100 to see the lint scripts ==="
sed -n '85,100p' .vortex/installer/composer.json
echo ""
echo "=== Full composer.json for context ==="
cat .vortex/installer/composer.jsonLength of output: 4320
🌐 Web query:
Rector 2.2.8 caching behavior --clear-cache flag required
💡 Result:
Short answer: Yes — Rector caches processed files, so to force a full re-run (e.g., after changing rules/config or when files were previously skipped) you must clear the cache (use --clear-cache). Many docs and reports recommend always running with --clear-cache when changing rules or running on a full codebase; Rector’s behaviour also includes kaizen/cache rules that enforce clearing when files or rules change. [1][2][3]
Sources:
- Rector release notes / changelog (mentions kaizen / cache behaviour). [1]
- Neos/rector docs (explicit recommendation: run with --clear-cache after adjusting rules). [2]
- Community answers / examples showing vendor/bin/rector process --clear-cache usage (StackOverflow / blog posts). [3]
Restore --clear-cache flags to Rector commands for consistent linting results.
Rector caches processed files and recommends using --clear-cache to force a full re-run, especially when changing rules or running on a full codebase. Removing this flag from both lint scripts (lines 91 and 94) risks inconsistent or stale cache-based results.
Restore --clear-cache to:
- Line 91:
"rector --clear-cache --dry-run" - Line 94:
"rector --clear-cache"
Also applies to: 94-94
🤖 Prompt for AI Agents
.vortex/installer/composer.json around lines 91 and 94: the Rector commands
removed the --clear-cache flag causing potential stale cache results; restore
the flag by changing the entry at line 91 to include --clear-cache --dry-run and
the entry at line 94 to include --clear-cache so Rector always clears its cache
before running.
| // Process each module that was NOT selected (remove them). | ||
| foreach (array_keys($all_modules) as $module_name) { | ||
| if (!in_array($module_name, $selected_modules)) { | ||
| // Remove from composer.json. | ||
| $pattern = '/\s*"drupal\/' . preg_quote($module_name, '/') . '":\s*"[^\"]+",?\n/'; | ||
| File::replaceContentInFile($t . '/composer.json', $pattern, "\n"); | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick | 🔵 Trivial
Make composer.json removal regex resilient to CRLF line endings (optional)
The pattern:
$pattern = '/\s*"drupal\/' . preg_quote($module_name, '/') . '":\s*"[^\"]+",?\n/';assumes \n line endings and may fail to match lines ending with \r\n. To make this more robust across platforms without changing behavior on LF files, consider allowing an optional \r:
- $pattern = '/\s*"drupal\/' . preg_quote($module_name, '/') . '":\s*"[^\"]+",?\n/';
+ $pattern = '/\s*"drupal\/' . preg_quote($module_name, '/') . '":\s*"[^\"]+",?(\r?\n)/';and keep the replacement consistent (e.g., "\n" or "$1" depending on how you want to normalize line endings).
🤖 Prompt for AI Agents
.vortex/installer/src/Prompts/Handlers/Modules.php around lines 75 to 81: the
regex used to remove module lines from composer.json assumes LF-only line
endings and will miss CRLF lines; update the pattern to accept an optional
carriage return (use \r?\n) so it matches both "\n" and "\r\n", and ensure the
replacement uses the desired normalized newline (e.g., "\n") so file line
endings remain consistent after removal.
Closes #2014
Summary by CodeRabbit
New Features
Improvements
Chores
Tests