feat(legacy): update deprecated PHP code #2789
Conversation
mp3butcher
changed the title
mp3butcher
changed the title
There was a problem hiding this comment.
These all look reasonable. Can you please add CI runs against PHP 8.x here and fix the linting failures?
mp3butcher
force-pushed
the
ubuntu22.04
branch
from
b7f0e9f
to
666f3f9
Compare
|
@paddatrapper Hi,
|
Ah, I misunderstood the purpose of this PR and thought you were adding support for PHP 8.
The failing 7.4 PHP linting CI shows diffs on formatting changes that need to be applied to your code. You can also test this locally via pre-commit. For example the following change highlights a space that needs removing: --- /home/runner/work/libretime/libretime/legacy/application/services/MediaService.php
+++ /home/runner/work/libretime/libretime/legacy/application/services/MediaService.php
@@ -135,7 +135,7 @@
*/
public static function areFilesStuckInPending()
{
- $oneHourAgo = gmdate( DEFAULT_TIMESTAMP_FORMAT, floor(hrtime(true)) - self::PENDING_FILE_TIMEOUT_SECONDS);
+ $oneHourAgo = gmdate(DEFAULT_TIMESTAMP_FORMAT, floor(hrtime(true)) - self::PENDING_FILE_TIMEOUT_SECONDS);
self::$_pendingFiles = CcFilesQuery::create()
->filterByDbImportStatus(CcFiles::IMPORT_STATUS_PENDING)
->filterByDbUtime($oneHourAgo, Criteria::LESS_EQUAL) |
mp3butcher
force-pushed
the
ubuntu22.04
branch
2 times, most recently
from
ac2906e
to
4e509dd
Compare
There was a problem hiding this comment.
gmdate clearly wait an int here...
my concern is microtime is used multiple times in libretime.
Do int or float is waited for these calls?
In brief, do I have to change them all to intval(microtime())?...
There was a problem hiding this comment.
@jooola do you have any insight here?
There was a problem hiding this comment.
this can be subject of an other pr, as it works perfectely without further changes
mp3butcher
force-pushed
the
ubuntu22.04
branch
3 times, most recently
from
24ec6ec
to
831c9c5
Compare
mp3butcher
changed the title
mp3butcher
changed the title
mp3butcher
changed the title
mp3butcher
force-pushed
the
ubuntu22.04
branch
4 times, most recently
from
4f28717
to
cac7f6e
Compare
mp3butcher
force-pushed
the
ubuntu22.04
branch
2 times, most recently
from
98c65c7
to
1d85653
Compare
|
There are a lot of resources in regard to the PHP 8.0 support, I think it's wise to take a look at them and use the existing work of others: #2047 |
|
This PR is not testable, the docker dev setup does not work out of the box when bumping the php version to 8.1. Please make sure to fix this, so this PR is self contained. |
|
I thought the point of this PR was that is isn't upgrading PHP to 8.0. Rather it's a prerequisite for that to happen later |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2789 +/- ##
=======================================
Coverage 70.36% 70.36%
=======================================
Files 149 149
Lines 4033 4033
=======================================
Hits 2838 2838
Misses 1195 1195
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
All the changes under Please propose a change against the propel library and we can then update the files by regenerating them here. Other than that, the rest looks good. |
|
Maybe enabling https://www.php.net/manual/en/language.types.declarations.php#language.types.declarations.strict in our files is also a good idea? |
|
i pushed a commit on libretime/propel1#2 but i'm not fixing this pr since you take over ..... |
Description
update deprecated code. It's mergeable with master without syntax conflicts across php versions
remove deprecated((https://www.php.net/manual/fr/function.strftime.php)) and unsafe (https://cve.mitre.org/cgi-bin/cvekey.cgi?keyword=strftime) strftime syntax