D11-11: Bump core version, and address some issues flagged by upgrade_status#169
D11-11: Bump core version, and address some issues flagged by upgrade_status#169adam-vessey wants to merge 10 commits intomainfrom
Conversation
📝 WalkthroughWalkthroughUpdate Composer package constraints and Drush services constraint; widen Drupal core compatibility in many module manifests and add a few module dependencies; adjust test visibility and assertion; replace a database prefix call; and add return type declarations to several __sleep() methods. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
adam-vessey
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@composer.json`:
- Line 34: The current Composer platform config entry for
"drush.11-12.services.yml" uses an open-ended constraint ">=11.6"; update this
to a bounded version to avoid accidentally allowing incompatible Drush major
releases (e.g., change the constraint to ">=11.6,<14.0" or another agreed upper
bound) and document the chosen bound in a brief comment or commit message;
locate the key "drush.11-12.services.yml" in composer.json and replace the
version string accordingly, ensuring composer validate passes and running a
quick compatibility test (e.g., install or update) to confirm no regressions.
| "drush": { | ||
| "services": { | ||
| "drush.11-12.services.yml": "^11.6 || ^12", | ||
| "drush.11-12.services.yml": ">=11.6", |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Drush 13 release Drupal 11 compatibility
💡 Result:
- Drush 13.0.0 release date: August 2, 2024. [1]
- Drupal 11 compatibility: Yes — Drush 13 is marked as supported/recommended for Drupal 11 (and compatible with Drupal 10.2+). [2]
- Key requirement: Drush 13 requires PHP 8.3+. [2]
Sources:
[1] endoflife.date (Drush release timeline)
[2] Drush docs (Drupal compatibility table)
Revisit the open-ended Drush services constraint now that Drush 13 is released.
Drush 13 was released in August 2024 and is compatible with Drupal 11. The constraint >=11.6 does match Drush 13 as intended, but given the commit message indicated this was a "temporary/compatibility change," the constraint approach should be formally reassessed. Determine whether this open-ended constraint will remain appropriate for Drush 14+ or if it requires tightening to a specific upper bound (e.g., >=11.6,<14.0).
🤖 Prompt for AI Agents
In `@composer.json` at line 34, The current Composer platform config entry for
"drush.11-12.services.yml" uses an open-ended constraint ">=11.6"; update this
to a bounded version to avoid accidentally allowing incompatible Drush major
releases (e.g., change the constraint to ">=11.6,<14.0" or another agreed upper
bound) and document the chosen bound in a brief comment or commit message;
locate the key "drush.11-12.services.yml" in composer.json and replace the
version string accordingly, ensuring composer validate passes and running a
quick compatibility test (e.g., install or update) to confirm no regressions.
Seems appropriate for now? Though, I suppose it is to be removed for Drush 14? Shrug?
Adding version check for `checkStatus()` parent function.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/MigrateBatchExecutable.php`:
- Around line 481-485: There's an extra blank line after the ternary assignment
to $status causing lint failures; edit the method in MigrateBatchExecutable.php
where $status is set using version_compare(... ) ?
MigrationInterface::RESULT_COMPLETED : parent::checkStatus() and remove the
consecutive empty line(s) so the assignment and the following code are
contiguous (no extra blank lines).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5d4bd82c-db09-40f0-b2b2-7f2167c5a68d
📒 Files selected for processing (1)
src/MigrateBatchExecutable.php
| $status = version_compare(\Drupal::VERSION, '11.3.0', '>=') ? | ||
| MigrationInterface::RESULT_COMPLETED : | ||
| parent::checkStatus(); | ||
|
|
||
|
|
There was a problem hiding this comment.
Fix linting error: Remove extra empty line.
The pipeline is failing due to multiple consecutive empty lines (lines 484-485).
Proposed fix
protected function checkStatus() {
$status = version_compare(\Drupal::VERSION, '11.3.0', '>=') ?
MigrationInterface::RESULT_COMPLETED :
parent::checkStatus();
-
if ($status === MigrationInterface::RESULT_COMPLETED) {🧰 Tools
🪛 GitHub Actions: Code Linting
[error] 484-484: Functions must not contain multiple empty lines in a row; found 2 empty lines (Squiz.WhiteSpace.SuperfluousWhitespace.EmptyLines).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/MigrateBatchExecutable.php` around lines 481 - 485, There's an extra
blank line after the ternary assignment to $status causing lint failures; edit
the method in MigrateBatchExecutable.php where $status is set using
version_compare(... ) ? MigrationInterface::RESULT_COMPLETED :
parent::checkStatus() and remove the consecutive empty line(s) so the assignment
and the following code are contiguous (no extra blank lines).
3 participants
Builds on #168
Summary by CodeRabbit