feat: Laravel 13 support#122
Conversation
- allow illuminate/contracts ^13.0 (keeps ^11/^12) - widen dev deps for the L13 toolchain: pest ^4, larastan ^3, phpstan-deprecation-rules ^2, phpstan-phpunit ^2 - drop jordanpartridge/conduit-interfaces: unused in code (comments/test strings only) and its laravel-zero ^11|^12 requirement caps the tree at Laravel 12 - add Laravel 13 (testbench 11) to the CI matrix, PHP 8.3+
phpstan.neon.dist includes phpstan-baseline.neon but the file was deleted in #111, breaking every phpstan run. Regenerated against larastan 3 (9 pre-existing errors baselined).
📝 WalkthroughWalkthroughThe project now accepts additional Laravel and development-tool versions, adds Laravel 13 to the GitHub Actions test matrix, excludes unsupported PHP combinations, and records six PHPStan baseline suppressions. ChangesLaravel compatibility
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
There was a problem hiding this comment.
Ship it.
- Removed dead
jordanpartridge/conduit-interfacesdep that blocked Laravel 13 (confirmed no src/ or test/ references). - Test matrix extended to L13 with PHP 8.3/8.4; PHP 8.2 exclusion correct.
- phpstan baseline regenerated against larastan 3; all pre-existing errors captured.
- Local run (Pest + phpstan) clean on 13.19 / 8.4.
L11/L12 unaffected. Ready to merge + tag.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
composer.json (1)
39-39: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider trimming stale
orchestra/testbenchconstraints.The constraint includes
^8.0(Laravel 10, dropped from the test matrix) and^12.0(no corresponding Laravel version in the matrix). Whileprefer-stableprevents accidental installs of non-existent versions, tightening to^9.0||^10.0||^11.0would align with the actual tested versions and reduce confusion.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@composer.json` at line 39, Update the orchestra/testbench dependency constraint in composer.json to remove the stale ^8.0 and ^12.0 ranges, retaining only ^9.0||^10.0||^11.0 to match the supported test matrix.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/run-tests.yml:
- Around line 31-36: Remove the php 8.2 and laravel 12.* entry from the exclude
matrix in the workflow, while retaining the php 8.2 and laravel 13.* exclusion.
In `@phpstan-baseline.neon`:
- Around line 33-37: Remove the stale class.notFound entry from
phpstan-baseline.neon, then update the connector() PHPDoc in the Github facade
to reference an existing return type, or restore the missing
GithubConnectorInterface dependency if it is required.
- Around line 9-31: Fix GithubConnector exception handling: define or correctly
reference ResourceNotFoundException so the 404 branch can throw a valid class,
pass the required message argument instead of reason when constructing
NetworkException for 5xx responses, and remove the redundant === '' comparison
after the existing empty-value guard.
---
Nitpick comments:
In `@composer.json`:
- Line 39: Update the orchestra/testbench dependency constraint in composer.json
to remove the stale ^8.0 and ^12.0 ranges, retaining only ^9.0||^10.0||^11.0 to
match the supported test matrix.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 76d90a77-1b97-4eb3-be34-b150007b2430
📒 Files selected for processing (3)
.github/workflows/run-tests.ymlcomposer.jsonphpstan-baseline.neon
| exclude: | ||
| # Laravel 12 requires PHP 8.3+ | ||
| # Laravel 12+ requires PHP 8.3+ | ||
| - php: 8.2 | ||
| laravel: 12.* | ||
| - php: 8.2 | ||
| laravel: 13.* |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
What is the minimum PHP version required for Laravel 12?
💡 Result:
The minimum PHP version required for Laravel 12 is PHP 8.2 [1][2][3]. According to the official Laravel documentation, the framework requires PHP >= 8.2 [1]. Additionally, official release notes indicate that Laravel 12 maintains support for PHP versions 8.2 through 8.5 [4][5].
Citations:
- 1: https://laravel.com/docs/12.x/deployment
- 2: https://laravel-news.com/laravel-12
- 3: https://itmarkerz.co.in/blog/laravel-php-compatibility-matrix
- 4: https://laravel.com/docs/12.x/releases
- 5: https://laravel.com/docs/master/releases
Drop the Laravel 12 / PHP 8.2 exclusion Laravel 12 supports PHP 8.2+, so excluding php: 8.2 for laravel: 12.* removes a valid test combination. Keep the 8.2 exclusion only for Laravel 13.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/run-tests.yml around lines 31 - 36, Remove the php 8.2 and
laravel 12.* entry from the exclude matrix in the workflow, while retaining the
php 8.2 and laravel 13.* exclusion.
| - | ||
| message: '#^Instantiated class JordanPartridge\\GithubClient\\Exceptions\\ResourceNotFoundException not found\.$#' | ||
| identifier: class.notFound | ||
| count: 1 | ||
| path: src/Connectors/GithubConnector.php | ||
|
|
||
| - | ||
| message: '#^Missing parameter \$message \(string\) in call to JordanPartridge\\GithubClient\\Exceptions\\NetworkException constructor\.$#' | ||
| identifier: argument.missing | ||
| count: 1 | ||
| path: src/Connectors/GithubConnector.php | ||
|
|
||
| - | ||
| message: '#^Strict comparison using \=\=\= between non\-falsy\-string and '''' will always evaluate to false\.$#' | ||
| identifier: identical.alwaysFalse | ||
| count: 1 | ||
| path: src/Connectors/GithubConnector.php | ||
|
|
||
| - | ||
| message: '#^Unknown parameter \$reason in call to JordanPartridge\\GithubClient\\Exceptions\\NetworkException constructor\.$#' | ||
| identifier: argument.unknown | ||
| count: 1 | ||
| path: src/Connectors/GithubConnector.php |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect the GithubConnector and NetworkException to verify the defects
ast-grep outline src/Connectors/GithubConnector.php --items all
rg -n 'ResourceNotFoundException' src/ --type=php
rg -n 'NetworkException' src/ --type=php -A2
rg -n '===' src/Connectors/GithubConnector.phpRepository: jordanpartridge/github-client
Length of output: 2951
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== NetworkException =="
cat -n src/Exceptions/NetworkException.php
echo
echo "== GithubConnector relevant sections =="
sed -n '130,170p' src/Connectors/GithubConnector.php
echo
echo "== token handling =="
sed -n '60,90p' src/Connectors/GithubConnector.phpRepository: jordanpartridge/github-client
Length of output: 4276
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== ResourceNotFoundException definitions =="
rg -n 'class ResourceNotFoundException|interface ResourceNotFoundException|trait ResourceNotFoundException' src/ tests/ --type=php || true
echo
echo "== GithubConnector constructor/property context =="
sed -n '1,70p' src/Connectors/GithubConnector.phpRepository: jordanpartridge/github-client
Length of output: 2354
Fix the broken exception paths in src/Connectors/GithubConnector.php.
ResourceNotFoundExceptionis imported and thrown on 404, but there’s no class definition in the tree, so that path will error when hit.NetworkExceptionexpectsmessage:; passingreason:on 5xx responses will error before the intended exception is raised.- The
=== ''check is redundant after the preceding empty-value guard.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@phpstan-baseline.neon` around lines 9 - 31, Fix GithubConnector exception
handling: define or correctly reference ResourceNotFoundException so the 404
branch can throw a valid class, pass the required message argument instead of
reason when constructing NetworkException for 5xx responses, and remove the
redundant === '' comparison after the existing empty-value guard.
| - | ||
| message: '#^PHPDoc tag @method for method JordanPartridge\\GithubClient\\Facades\\Github\:\:connector\(\) return type contains unknown class JordanPartridge\\GithubClient\\Contracts\\GithubConnectorInterface\.$#' | ||
| identifier: class.notFound | ||
| count: 1 | ||
| path: src/Facades/Github.php |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if GithubConnectorInterface exists in the codebase or was from conduit-interfaces
rg -n 'GithubConnectorInterface' src/ --type=php
fd 'conduit-interfaces' --type d
rg -n 'conduit-interfaces' composer.jsonRepository: jordanpartridge/github-client
Length of output: 288
Remove the stale class.notFound suppression
src/Facades/Github.php still declares connector() as returning JordanPartridge\GithubClient\Contracts\GithubConnectorInterface, but that type isn't defined in this repo. Either restore the dependency or update the PHPDoc to a real return type.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@phpstan-baseline.neon` around lines 33 - 37, Remove the stale class.notFound
entry from phpstan-baseline.neon, then update the connector() PHPDoc in the
Github facade to reference an existing return type, or restore the missing
GithubConnectorInterface dependency if it is required.
Quality Gate: errorNo code quality violations found in the "jordanpartridge/github-client" repository. The "jordanpartridge/github-client" repository has been audited for code quality using multiple tools including pint, pest, phpstan, and rector. All these tools have reported no violations, indicating that the codebase is clean and adheres to best practices as per the configured standards. This review suggests that the repository's code quality is robust and up-to-date with respect to coding standards. |
Constraint changes
^11.0||^12.0^11.0||^12.0||^13.0^1.1^2.0^2.0||^3.0^2.34||^3.0^2.34||^3.0||^4.0^1.1^1.1||^2.0^1.3^1.3||^2.0Why conduit-interfaces was removed: it requires
laravel-zero/framework ^11|^12+illuminate/console ^11|^12, which makes the dependency tree unresolvable on Laravel 13 (laravel-zero has no v13 release yet). The package is not referenced anywhere insrc/ortests/— only in comments and test fixture strings — so it was a dead dependency blocking the upgrade.phpstan baseline:
phpstan.neon.distincludesphpstan-baseline.neon, but that file was deleted in #111, breaking every phpstan run since. Regenerated against larastan 3 (9 pre-existing errors baselined).CI
Local test results (Laravel 13.19.0 / testbench 11.1.0 / pest 4.7.5, PHP 8.4)
Laravel 11/12 support unchanged. No release tagged — merge and tag at your discretion.
Summary by CodeRabbit
Compatibility
Testing
Code Quality