-
Notifications
You must be signed in to change notification settings - Fork 16
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: add php8.4 support #248
Conversation
Warning Rate limit exceeded@taka-oyama has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 24 minutes and 5 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughThis pull request introduces updates to support newer PHP versions across multiple files. The changes include updating the Dockerfile to use PHP 8.4, expanding the PHP version compatibility in Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
Dockerfile
(1 hunks)composer.json
(1 hunks)src/Connection.php
(1 hunks)src/Schema/Builder.php
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Run-PHPUnit
🔇 Additional comments (3)
Dockerfile (1)
1-1
: Verify PHP 8.4 stability and extension compatibilityPHP 8.4 is currently in development/RC stage. Consider the following:
- Ensure all required extensions (grpc, protobuf, pcov) are compatible with PHP 8.4
- Consider waiting for the stable release before using in production
✅ Verification successful
PHP 8.4 usage is aligned with project requirements
The project's
composer.json
explicitly declares support for PHP 8.4 ("php": "^8.2|^8.3|^8.4"
). As this is a library package, early adoption of PHP 8.4 allows consumers to test compatibility with the upcoming PHP version. Required extensions are specified without version constraints, suggesting flexibility across PHP versions.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check PHP version and extension compatibility docker run --rm php:8.4-cli-alpine php -v docker run --rm php:8.4-cli-alpine pecl list-all | grep -E "grpc|protobuf|pcov"Length of output: 319
Script:
#!/bin/bash # Check PHP version requirements and extensions fd composer.json --exec cat {} \; fd "composer.*|.*\\.json" --exec grep -l "php" {} \;Length of output: 1712
src/Connection.php (1)
100-107
: LGTM! Type safety improvementsThe addition of explicit nullable type hints for
$authCache
and$sessionPool
parameters improves type safety and aligns with PHP's type system.src/Schema/Builder.php (1)
84-84
: LGTM! Enhanced type safetyThe addition of explicit nullable type hint for the
$callback
parameter improves type safety and maintains backward compatibility.
composer.json
Outdated
@@ -8,7 +8,7 @@ | |||
{"name": "Takayasu Oyama", "email": "[email protected]"} | |||
], | |||
"require": { | |||
"php": "^8.2", | |||
"php": "^8.2|^8.3|^8.4", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codebase verification
Remove PHP 8.4 from version constraints
PHP 8.4 is not yet released, and Laravel 11.x explicitly requires PHP ^8.2. Adding PHP 8.4 to version constraints is premature and should be done after:
- Official PHP 8.4 release
- Laravel Framework adds support for PHP 8.4
- Major dependencies verify compatibility
🔗 Analysis chain
Verify dependency compatibility with PHP 8.4
While the PHP version requirement has been updated, please verify:
- Laravel 11.15.0 compatibility with PHP 8.4
- All other dependencies support PHP 8.4
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check Laravel's composer.json for PHP 8.4 support
curl -s https://raw.githubusercontent.com/laravel/framework/11.x/composer.json | jq '.require.php'
# Check other major dependencies
for pkg in "google/cloud-spanner" "grpc/grpc" "symfony/cache" "symfony/lock"; do
echo "Checking $pkg..."
curl -s "https://repo.packagist.org/p2/$pkg.json" | jq -r '.packages[][] | select(.version | startswith("1.")) | {version: .version, php: .require.php}'
done
Length of output: 2484
Summary by CodeRabbit
Release Notes
New Features
Improvements
Connection
constructorBuilder
method signature by allowing nullable callbackMaintenance