-
Notifications
You must be signed in to change notification settings - Fork 281
Support PHP 8.4/8.5 #5454
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
base: main
Are you sure you want to change the base?
Support PHP 8.4/8.5 #5454
Conversation
| { | ||
| $id = trim($id); | ||
| $timestamp = (int) $timestamp; | ||
| $timestamp = (int) floor($timestamp); |
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.
Please make absolutely sure we're dealing with floats here. My feeling is that this change isn't strictly necessary since we're dealing with timestamps and those are usually a result of time() or DateTime->getTimestamp(), in which case it's always an int.
There are probably other cases as well, I won't comment on each one. Please review them again and ensure on your own that you don't floor unnecessarily. It's maybe also an option to prevent decimals in the first place, given the calls are internal only. Though, an exception is Benchmark, as it already floored its timestamp previously so there's no problem to do it still.
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.
The change has been reversed.
modules/monitoring/library/Monitoring/Backend/Ido/Query/EventgridQuery.php
Outdated
Show resolved
Hide resolved
modules/monitoring/library/Monitoring/Backend/Ido/Query/IdoQuery.php
Outdated
Show resolved
Hide resolved
|
Also, the changes to the monitoring module seem implausible to be complete. You already acknowledged in person that you didn't test it yet since you don't have the IDO enabled/configured. Please talk to @lippserd whether that's necessary to begin with. |
|
The fact that 730af4c doesn't conditionally call |
04f618b to
bcade4e
Compare
…placed by Pdo\Mysql
…n PHP 8.5, calling it had no effect since PHP 8.1
…t's value is deprecated in PHP 8.4
…r value-altering float to int casts.
bcade4e to
d5c87b1
Compare
21293ec to
d5c87b1
Compare
Passing null to non nullable parameters of built-in functions is deprecated since PHP-8.1
53102a4 to
03995b3
Compare
- Remove all errors that were caused by the monitoring module as it is no longer a part if this repo - Remove baselines of php versions that are no longer supported - Merge baselines standard, 8.1 and 8.x since seperating them is no longer necessary. - Remove errors that are no longer generated
03995b3 to
be1b54a
Compare
sukhwinder33445
left a comment
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.
LGTM 👍🏽
Changes that had to be addressed
PHP 8.3 -> PHP 8.4
Migration docs: https://www.php.net/manual/en/migration84
session.sid_bits_per_characteris deprecated.PHP 8.4 -> PHP 8.5
Migration Docs: https://www.php.net/manual/en/migration85
instead of PDO::SQL<constant_name> use Pdo\Mysql::<constant_name>.
resolves: #5269
resolves: #5450