-
Notifications
You must be signed in to change notification settings - Fork 4
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
[PHP 8.5] Use PHP_BUILD_DATE
constant
#65
Conversation
PHP 8.5 has a [new constant named `PHP_BUILD_DATE`](https://php.watch/versions/8.5/PHP_BUILD_DATE) that contains the same value as the `phpinfo` build date. We currently use "Build date" field parsed from `phpinfo` output, but on PHP 8.5, we can simply return the `PHP_BUILD_DATE` value.
WalkthroughThe changes involve a modification to the Changes
Poem
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: 0
🧹 Outside diff range and nitpick comments (2)
src/Debug/Utility/Php.php (2)
39-41
: Documentation needs update to reflect PHP 8.5 supportThe method documentation should be updated to reflect the new PHP 8.5 functionality:
/** * Return the build date of the PHP binary + * + * As of PHP 8.5, uses the PHP_BUILD_DATE constant when available. + * For older versions, falls back to parsing phpinfo() output. * * @return string|null Returns null if build date cannot be determined + * @since 3.x.x Added PHP 8.5 PHP_BUILD_DATE constant support */
Line range hint
42-50
: Consider adding comments and optimizing phpinfo captureThe fallback implementation is functional but could be improved for clarity and efficiency:
+ // Fallback for PHP < 8.5: Extract build date from phpinfo output \ob_start(); - \phpinfo(INFO_GENERAL); + \phpinfo(\INFO_GENERAL); // Only capture general info section $phpInfo = \ob_get_clean(); $phpInfo = \strip_tags($phpInfo); \preg_match('/Build Date (?:=> )?([^\n]*)/', $phpInfo, $matches); return $matches ? $matches[1] : null;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
src/Debug/Utility/Php.php
(1 hunks)
🔇 Additional comments (2)
src/Debug/Utility/Php.php (2)
39-41
: LGTM! Clean implementation of PHP 8.5 constant support
The implementation correctly checks for and uses the new PHP_BUILD_DATE
constant, following PHP best practices with proper constant scoping and early return pattern.
Line range hint 39-50
: Verify test coverage and usage
Let's ensure this change is properly covered:
✅ Verification successful
Based on the search results, I can see that:
-
The
buildDate
method is used in only 3 files:- src/Debug/Utility/Php.php (implementation)
- src/Debug/Plugin/LogPhp.php (usage)
- tests/Debug/Plugin/LogPhpTest.php (test)
-
The test file exists and the changes appear to be covered by tests.
-
The method is only used internally by the LogPhp plugin for debugging purposes.
Let me verify the test coverage and implementation:
Changes to buildDate method are safe and well-tested
The changes to the buildDate
method are safe because:
- The method is only used internally by the LogPhp plugin for debug output
- The changes maintain backward compatibility by first checking for the constant
- The implementation is covered by tests that handle both cases (when build date is available and when it's null)
- The usage in LogPhp plugin properly handles the null case by conditionally logging the build date
The changes follow a proper fallback pattern and the test coverage confirms the behavior is working as expected.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for existing tests and usage of buildDate method
echo "Checking for test files..."
fd -e php -e phpt test
echo "Checking for buildDate method usage..."
rg -l "buildDate" --type php
echo "Checking for existing PHP version checks..."
rg "PHP_VERSION|defined\(" --type php
Length of output: 33341
Script:
#!/bin/bash
# Check the test implementation
echo "=== Test Implementation ==="
rg -A 10 "buildDate" tests/Debug/Plugin/LogPhpTest.php
echo -e "\n=== Usage in LogPhp Plugin ==="
rg -A 5 "buildDate" src/Debug/Plugin/LogPhp.php
Length of output: 1060
This is great! (I opened php/php-src#16560) |
PHP 8.5 has a new constant named
PHP_BUILD_DATE
that contains the same value as thephpinfo
build date.We currently use "Build date" field parsed from
phpinfo
output, but on PHP 8.5, we can simply return thePHP_BUILD_DATE
value.Summary by CodeRabbit
New Features
buildDate
method to return the PHP build date directly if available, improving efficiency.Bug Fixes
phpinfo()
by introducing a new conditional check.