Fix #42: declare the laravel/framework dependency the package already has - #88
Open
corgab wants to merge 2 commits into
Open
Fix #42: declare the laravel/framework dependency the package already has#88corgab wants to merge 2 commits into
corgab wants to merge 2 commits into
Conversation
src/ imports Illuminate\Foundation (AboutCommand, PendingDispatch, Foundation\Queue\Queueable) and calls the config(), app(), event(), report() and dispatch() helpers, all of which ship only inside laravel/framework: Foundation is not published as a standalone illuminate/* component, so the illuminate/* entries alone did not describe what the package needs. composer.json now requires laravel/framework ^13.0 alongside them, and a manifest test fails if an Illuminate namespace is imported without a package that provides it. Closes #42
With laravel/framework required, the four illuminate/* entries only duplicated what the framework already replaces, and the CI matrix pinned those components instead of the framework that actually gets installed; the matrix now pins laravel/framework per Laravel version. The manifest test asserts the requirement itself, not its version constraint, and checks that src still uses Illuminate\Foundation classes or helpers, which is what justifies it.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
composer.jsonrequired onlyilluminate/console,contracts,databaseandsupport, yetsrc/importsIlluminate\Foundation\Console\AboutCommand,Illuminate\Foundation\Bus\PendingDispatchandIlluminate\Foundation\Queue\Queueable, and calls theconfig(),app(),event(),report(),dispatch(),base_path()andconfig_path()helpers. Those all live inlaravel/framework'sIlluminate\Foundation, so the manifest did not describe what the package needs.Plan
The issue suggested adding
illuminate/foundation,queue,bus,cacheandevents.illuminate/foundationdoes not exist:laravel/framework'sreplacelist covers every split component except Foundation, which is only shipped inside the framework itself. Since the package genuinely depends on Foundation classes and helpers, the accurate declaration islaravel/framework: ^13.0. That single entry also provides everyilluminate/*component, so the four previous entries are dropped rather than kept as duplicates, and the CI matrix pins the framework itself.Changes
composer.json:requireis nowphp,laravel/framework: ^13.0,symfony/process..github/workflows/tests.yml: the matrix step pinslaravel/framework:${{ matrix.laravel }}instead of the four components, so the version under test is the one actually installed and a future14.*row cannot conflict with a^13.0framework pin.tests/Unit/ComposerManifestTest.php(new): assertslaravel/frameworkis required (no coupling to the constraint string), and thatsrc/still usesIlluminate\Foundationclasses or the Foundation helpers, which is what justifies the requirement. The first test fails onmain.CLAUDE.md: one conventions bullet.No behaviour change, no new runtime code.
composer.lockis not tracked by the repository.Tests
Local: pint passed, pest 793 tests passed,
composer validatepasses on the manifest.Closes #42