-
-
Notifications
You must be signed in to change notification settings - Fork 28
Fixed installer to correctly update composer.json for Drupal CMS starter.
#2041
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
Fixed installer to correctly update composer.json for Drupal CMS starter.
#2041
Conversation
WalkthroughUpdates an embedded Composer configuration in the Starter handler: bumps Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested labels
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: ASSERTIVE Plan: Pro ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
⏰ 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). (14)
🔇 Additional comments (2)
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: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
.vortex/installer/src/Prompts/Handlers/Starter.php(1 hunks)
⏰ 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). (14)
- 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 (2)
- GitHub Check: vortex-test-workflow (0)
- GitHub Check: vortex-test-workflow (1)
- GitHub Check: vortex-test-deployment (0)
- GitHub Check: vortex-test-deployment (1)
- GitHub Check: vortex-test-common
- GitHub Check: vortex-test-installer (8.3)
- GitHub Check: vortex-test-installer (8.4)
- GitHub Check: vortex-test-installer (8.2)
- GitHub Check: vortex-test-docs
🔇 Additional comments (2)
.vortex/installer/src/Prompts/Handlers/Starter.php (2)
100-106: LGTM! Comprehensive allow-plugins configuration.The added plugin permissions are standard for Drupal CMS installations:
composer/installers: Handles custom install paths for Drupal packagesdrupal/core-composer-scaffold: Scaffolds Drupal core filesdrupal/core-recipe-unpack: Unpacks Drupal recipes (required for CMS)drupal/core-vendor-hardening: Hardens vendor directoryphp-http/discovery: HTTP client discoveryAll entries are correctly set to
TRUEand align with Drupal CMS ^1.2 requirements.
96-96: Approve drupal/cms ^1.2 bump. Verified 1.2.6 is the latest stable release with no breaking changes; ensure upgrade usescomposer update --with-all-dependenciesand run recipe unpack steps.
| $cj->addConfigSetting('allow-plugins.php-http/discovery', TRUE); | ||
| $cj->addConfigSetting('allow-plugins.wikimedia/composer-merge-plugin', TRUE); | ||
|
|
||
| $cj->addProperty('minimum-stability', 'alpha'); |
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
🧩 Analysis chain
Document the rationale for alpha minimum-stability.
Setting minimum-stability to alpha allows installation of unstable packages, which introduces risk. Please verify:
- Is this required for drupal/cms ^1.2 or its dependencies?
- Should this be conditional or temporary?
- Are there specific alpha packages that necessitate this setting?
Consider adding a comment in the code explaining why alpha stability is required.
🏁 Script executed:
#!/bin/bash
# Description: Check if drupal/cms ^1.2 requires alpha stability or has alpha dependencies
# Check drupal/cms composer.json for stability requirements
curl -s "https://repo.packagist.org/p2/drupal/cms.json" | \
jq '.packages["drupal/cms"][] | select(.version | startswith("1.2")) | {version, "minimum-stability": .["minimum-stability"], require}'Length of output: 5289
Remove or document global minimum-stability: alpha
- drupal/cms ^1.2 declares its pre-release deps with explicit
@alpha/@betaflags, so this global setting isn’t needed—drop it or replace withprefer-stable: true. - If you must keep it, add a comment explaining which specific package(s) require alpha stability.
🤖 Prompt for AI Agents
.vortex/installer/src/Prompts/Handlers/Starter.php around line 108: the code
sets a global "minimum-stability: alpha" which is unnecessary for drupal/cms (it
uses explicit @alpha/@beta) and is risky; either remove the global
minimum-stability line entirely or replace it with "prefer-stable: true"; if you
must keep "minimum-stability: alpha" add a clear inline comment naming the
specific package(s) that require alpha and why, and ensure the composer output
prefers stable releases by adding prefer-stable: true.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #2041 +/- ##
===========================================
+ Coverage 63.95% 63.99% +0.03%
===========================================
Files 91 91
Lines 5637 5643 +6
Branches 44 44
===========================================
+ Hits 3605 3611 +6
Misses 2032 2032 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
f4fd388 to
c478884
Compare
Summary by CodeRabbit