Laravel 13.x Compatibility#154
Conversation
|
|
|
|
Reviewer's guide (collapsed on small PRs)Reviewer's GuideUpdates package constraints and CI test matrix to add compatibility and coverage for Laravel 13 while excluding unsupported PHP/Laravel combinations. Flow diagram for version compatibility with Laravel 13graph TD
Start["Start: select PHP and Laravel versions"] --> CheckPHP["Check PHP version"]
CheckPHP -->|"PHP < 8.1"| OldCompat["Laravel 5.1 - 12.x only"]
OldCompat --> EndOld["Laravel 13 not allowed"]
CheckPHP -->|"PHP >= 8.1"| CheckLaravel["Check Laravel version"]
CheckLaravel -->|"Laravel <= 12.x"| Compat12["Allowed by composer constraint ~5.1|6.*|7.*|8.*|9.*|10.*|11.*|12.*"]
Compat12 --> End12["Use existing compatibility"]
CheckLaravel -->|"Laravel 13.x"| Compat13["Allowed by new composer constraint ^13.0"]
Compat13 --> End13["Run tests in CI matrix for Laravel 13"]
CheckLaravel -->|"Laravel > 13.x"| NotAllowed["Not allowed by current constraints"]
NotAllowed --> EndNA["Update constraints required"]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
⚗️ Using this package? If you would like to help test these changes or believe them to be compatible, you may update your project to reference this branch. To do so, temporarily add Shift's fork to the {
"repositories": [
{
"type": "vcs",
"url": "https://github.com/laravel-shift/laravel-heyman.git"
}
]
}Then update your dependency constraint to reference this branch: {
"require": {
"imanghafoori/laravel-heyman": "dev-l13-compatibility",
}
}Finally, run: |
📝 WalkthroughWalkthroughThe pull request expands Laravel 13.x support by updating the CI/CD test matrix to include Laravel 13 with PHP version exclusions, and extending the Laravel version constraint in composer.json to explicitly include Laravel 13. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 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 |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The test matrix adds Laravel
13.*but omits12.*, which you still support incomposer.json; consider including Laravel 12 in the workflow matrix or dropping it from the constraints. - For Laravel
13.*, you currently exclude only some PHP versions in the matrix; double‑check Laravel 13’s minimum PHP version and adjust theexcludelist (or the PHP matrix) so you’re not running unsupported PHP/Laravel combinations.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The test matrix adds Laravel `13.*` but omits `12.*`, which you still support in `composer.json`; consider including Laravel 12 in the workflow matrix or dropping it from the constraints.
- For Laravel `13.*`, you currently exclude only some PHP versions in the matrix; double‑check Laravel 13’s minimum PHP version and adjust the `exclude` list (or the PHP matrix) so you’re not running unsupported PHP/Laravel combinations.
## Individual Comments
### Comment 1
<location path="composer.json" line_range="26" />
<code_context>
"require": {
"php": ">=7.1",
- "laravel/framework": "~5.1|6.*|7.*|8.*|9.*|10.*|11.*|12.*"
+ "laravel/framework": "~5.1|6.*|7.*|8.*|9.*|10.*|11.*|12.*|^13.0"
},
"require-dev": {
</code_context>
<issue_to_address>
**issue (bug_risk):** Check PHP version constraint against Laravel 13 minimum requirements.
The `php` constraint is still `>=7.1`, but Laravel 13 will require a much higher PHP version (likely PHP 8+). This mismatch can let Composer install `laravel/framework:^13.0` on environments that satisfy `>=7.1` but can’t actually run Laravel 13. Please update the `php` constraint to at least Laravel 13’s minimum PHP version, or adjust the Laravel 13 constraint so they stay aligned.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| "require": { | ||
| "php": ">=7.1", | ||
| "laravel/framework": "~5.1|6.*|7.*|8.*|9.*|10.*|11.*|12.*" | ||
| "laravel/framework": "~5.1|6.*|7.*|8.*|9.*|10.*|11.*|12.*|^13.0" |
There was a problem hiding this comment.
issue (bug_risk): Check PHP version constraint against Laravel 13 minimum requirements.
The php constraint is still >=7.1, but Laravel 13 will require a much higher PHP version (likely PHP 8+). This mismatch can let Composer install laravel/framework:^13.0 on environments that satisfy >=7.1 but can’t actually run Laravel 13. Please update the php constraint to at least Laravel 13’s minimum PHP version, or adjust the Laravel 13 constraint so they stay aligned.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/tests.yml (1)
21-72:⚠️ Potential issue | 🟠 MajorAdd Laravel 12. to the test matrix; it is declared in composer.json but missing from the workflow.*
The
composer.jsondeclares support for Laravel12.*(along with~5.1|6.*|7.*|8.*|9.*|10.*|11.*|^13.0), but the CI matrix excludes it. Update the matrix to include this declared version:Suggested matrix adjustment
- laravel: ['6.*', '7.*', '8.*', '9.*', '10.*', '11.*', '13.*'] + laravel: ['6.*', '7.*', '8.*', '9.*', '10.*', '11.*', '12.*', '13.*']🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/tests.yml around lines 21 - 72, The CI matrix is missing Laravel 12.* despite composer.json declaring support; update the workflow's matrix entry named "laravel" to include "12.*" alongside the existing versions and ensure any relevant exclude blocks that mistakenly omit combinations with php versions are adjusted (remove or add excludes referencing laravel: 12.* as needed) so that valid PHP/Laravel combos for 12.* are tested; locate the "laravel" matrix list and the "exclude" blocks in the tests workflow and add "12.*" to the versions list and reconcile excludes to reflect supported PHP versions for Laravel 12.*.
🧹 Nitpick comments (1)
composer.json (1)
26-26: Consider replacinglaravel/frameworkwith minimalilluminate/*requirements (optional).This package uses components across multiple Illuminate subsystems (auth, routing, eloquent, events, validation, views, support). While technically feasible to split into individual
illuminate/*dependencies, the full framework requirement is reasonable here since this is an application-level package designed to integrate deeply with Laravel, and consuming applications will have the full framework installed anyway. Refactoring to granular dependencies would add complexity tocomposer.jsonwithout meaningful benefit.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@composer.json` at line 26, Leave the "laravel/framework" dependency as-is in composer.json (the current entry "laravel/framework": "~5.1|6.*|7.*|8.*|9.*|10.*|11.*|12.*|^13.0") rather than refactoring to individual illuminate/* packages; no code changes required—retain the full framework requirement because this is an application-level integration and consuming apps will provide the framework.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In @.github/workflows/tests.yml:
- Around line 21-72: The CI matrix is missing Laravel 12.* despite composer.json
declaring support; update the workflow's matrix entry named "laravel" to include
"12.*" alongside the existing versions and ensure any relevant exclude blocks
that mistakenly omit combinations with php versions are adjusted (remove or add
excludes referencing laravel: 12.* as needed) so that valid PHP/Laravel combos
for 12.* are tested; locate the "laravel" matrix list and the "exclude" blocks
in the tests workflow and add "12.*" to the versions list and reconcile excludes
to reflect supported PHP versions for Laravel 12.*.
---
Nitpick comments:
In `@composer.json`:
- Line 26: Leave the "laravel/framework" dependency as-is in composer.json (the
current entry "laravel/framework": "~5.1|6.*|7.*|8.*|9.*|10.*|11.*|12.*|^13.0")
rather than refactoring to individual illuminate/* packages; no code changes
required—retain the full framework requirement because this is an
application-level integration and consuming apps will provide the framework.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 65995a14-4878-4d12-bac9-7797f3f90fbb
📒 Files selected for processing (2)
.github/workflows/tests.ymlcomposer.json
1 participant
This is an automated pull request from Shift to update your package code and dependencies to be compatible with Laravel 13.x.
Before merging, you need to:
l13-compatibilitybranchIf you do find an issue, please report it by commenting on this PR to help improve future automation.
Summary by Sourcery
Add Laravel 13 compatibility to the package and its CI matrix.
New Features:
CI:
Summary by CodeRabbit
Tests
Chores