Skip to content

Register discovered package providers again - #255

Open
JonasPardon wants to merge 1 commit into
wintercms:wip/1.3from
JonasPardon:fix/discovered-package-providers
Open

JonasPardon wants to merge 1 commit into
wintercms:wip/1.3from
JonasPardon:fix/discovered-package-providers

Conversation

@JonasPardon

@JonasPardon JonasPardon commented Oct 7, 2026 •

Copy link
Copy Markdown

With app.loadDiscoveredPackages enabled, no discovered package provider gets registered on wip/1.3.

0a6b24e ("Laravel 10 changes") dropped the brackets around the manifest's providers in Application::registerConfiguredProviders():

$providers->splice(1, 0, $this->make(PackageManifest::class)->providers());

That splices each provider class in as a separate string, between the Illuminate\ group and the other providers. collapse() only flattens arrays and skips anything else, so those strings never reach ProviderRepository::load(). Laravel 12 still passes them as one group ([…->providers()]), which is what this restores. The default (loadDiscoveredPackages => false) is unaffected.

  • Restore the brackets.
  • Add a test that boots an Application with a discovered provider and checks that it gets registered. It fails without the change.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • Discovered package service providers are now registered correctly when the application loads configured providers, so their services are available as expected.

The package manifest's providers were spliced into the provider list as separate strings instead of as one group, so collapse() skipped them and no discovered provider was registered when app.loadDiscoveredPackages is enabled.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
@coderabbitai

coderabbitai Bot commented Oct 7, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: e10fc525-6580-47d2-abde-a3f83085ab37
📥 Commits

Reviewing files that changed from the base of the PR and between 16bdbbe and b5dba97.

📒 Files selected for processing (2)
  • src/Foundation/Application.php
  • tests/Foundation/ApplicationTest.php

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.


Walkthrough

Application::registerConfiguredProviders now passes discovered package providers to Collection::splice inside an array. A new test verifies that the discovered provider is registered and binds discovered-package. The test removes its temporary storage directories in a finally block.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to b5dba

No actionable merge-blocking risk remains from the reviewed change.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 16.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: restoring registration of discovered package providers.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@JonasPardon

Copy link
Copy Markdown
Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Oct 7, 2026 •

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@mjauvin mjauvin self-assigned this Oct 7, 2026
@mjauvin mjauvin added the maintenance PRs that fix bugs, are translation changes or make only minor changes label Oct 7, 2026
@mjauvin mjauvin added this to the 1.3.0 milestone Oct 7, 2026
@JonasPardon JonasPardon mentioned this pull request Oct 8, 2026
4 of 16 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

maintenance PRs that fix bugs, are translation changes or make only minor changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants