-
Notifications
You must be signed in to change notification settings - Fork 172
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Add PHP 8.2 support to main branch, drop 8.0 support #8264
Conversation
It looks like laminos/diactoros has a version that works with 7.4 OR a version that works with 8.2.. so this might need to go to main. |
Update packages that were failing `composer install`, fix deprecated dynamic property setting in NDB_Page by only setting $cls->menu after checking if it exists. (It exists on NDB_Menu subclasses but not all NDB_Pages)
Unassigned from myself because the tests are passing now that we've decided to put it in main instead of 24.2 and someone else needs to review it. |
* php8.0-curl (for development instances only) | ||
|
||
* libapache2-mod-php8.0 | ||
* php8.2-curl (for development instances only) |
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.
This is now a requirement for prod I think.
Http\Client needs that.
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.
It wasn't changed in this PR, just the version.. can you send a PR to fix if that's wrong now?
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.
Thinking about it, it would be part of Guzzle dependencies.
@@ -140,13 +140,13 @@ class Raw extends Endpoint implements \LORIS\Middleware\ETagCalculator | |||
|
|||
$callback = function () use ($mincpath, $fullpath) { | |||
return shell_exec( | |||
"${mincpath}/bin/minctoraw -byte -unsigned -normalize $fullpath" | |||
"{$mincpath}/bin/minctoraw -byte -unsigned -normalize $fullpath" |
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.
Why switch from simple to complex syntax?
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.
I'm not sure what's simple or complex about ${foo}
vs {$foo}
but phan was complaining that the former was deprecated and saying the latter should be used instead.
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.
For information:
https://www.php.net/manual/en/language.types.string.php
Simple syntax
If a dollar sign ($) is encountered, the parser will greedily take as many tokens as possible to form a valid variable name. Enclose the variable name in curly braces to explicitly specify the end of the name.
Complex (curly) syntax
Any scalar variable, array element or object property with a string representation can be included via this syntax. The expression is written the same way as it would appear outside the string, and then wrapped in { and }. Since { can not be escaped, this syntax will only be recognized when the $ immediately follows the {.
So, both worked in this context but the former is deprecated in 8.2. Probably because it can be achieved with the latter.
Update packages that were failing
composer install
, fix deprecated dynamic property warning from NDB_Page by only setting $cls->menu after checking if it exists. (It exists on NDB_Menu subclasses but not all NDB_Pages.)I clicked around a few pages to make sure that LORIS works, but didn't thoroughly test it with PHP 8.2. So far it seems to have been much less painful than 8.0->8.1 and I haven't encountered anything that can't be fixed in a bugfix release.