PHP 8.2 compatibility fix #1553
Conversation
|
You said there are a lot of deprecation warnings (note, they're not errors) and the description of the pr makes it sound like there are multiple changes required - but there's only one. Can you please clarify if this pr is complete? |
|
Sorry, meant deprecation notices. I meant that silverstripe-admin causes deprecation notices on all requests to /admin, hence my wording of a lot of notices. The PR is complete though, with this change (which has no impact otherwise) we can get rid of these notices, so that we can fix deprecation notices in other modules. I see that there is a linting error from this change though, should the property perhaps be declared private in this case, since it is not used by any other code? |
|
It should be public, because the dynamic property was public implicitly. But in any case, the linting error is because of the underscore more than because of the visibility I think. You may need to add a phpdoc comment to disable the linting rule for that property. |
|
Changed the property to public, and disabled linting for that line |
|
I'll go ahead and change this PR to use the AllowDynamicProperties annotation instead of declaring the property public, as per silverstripe/silverstripe-framework#10921 |
annotation on classes that set dynamic properties
forsdahl
force-pushed
the
php8.2-compatibility
branch
from
518b8a7
to
0b59826
Compare
|
This will be automatically tagged when GitHub Actions has finished running on the branch. |
Currently silverstripe-admin in version 4 triggers a lot of deprecation errors, because of setting dynamic class properties. This can be easily fixed by declaring these properties on the classes, and should have no impact on the code otherwise.
The reason for fixing this is to be able to fix deprecation errors with PHP 8.2 in other Silverstripe modules without these errors getting buried in all the deprecation errors from silverstripe-admin, not to have version 4 of silverstripe-admin officially support PHP 8.2.