search

gjbarnard / moodle-format_topcoll

Collapsed Topics course format for Moodle.
http://docs.moodle.org/en/Collapsed_Topics_course_format
GNU General Public License v3.0
35 stars 60 forks source link

Deprecated warnings in PHP 8.2 #147

Closed PhMemmel closed 7 months ago

PhMemmel commented 7 months ago

Describe the bug Currently the code is producing deprecation warnings when running on PHP 8.2

To Reproduce For example run the unit tests via vendor/bin/phpunit --filter=courseformatrenderer_test. You will see:

Deprecated: Creation of dynamic property section_info::$toggle is deprecated in /var/www/html/course/format/topcoll/tests/courseformatrenderer_test.php on line 465

Deprecated: Creation of dynamic property section_info::$toggle is deprecated in /var/www/html/course/format/topcoll/classes/output/renderer.php on line 913

Deprecated: Creation of dynamic property section_info::$isshown is deprecated in /var/www/html/course/format/topcoll/classes/output/renderer.php on line 922

Expected behaviour No deprecation warnings.

gjb2048 commented 7 months ago

It's only a deprecation message (https://github.com/gjbarnard/moodle-format_topcoll/commit/3adebf9032fd161ef9fcc502409bb295ebea17e6) so not urgent at the moment.

gjb2048 commented 7 months ago

Looks like only need to worry when Moodle supports PHP 9.0 -> https://php.watch/versions/8.2/dynamic-properties-deprecated.

PhMemmel commented 7 months ago

Hi @gjb2048, thank you for your quick reply. I know that this is only a deprecation warning and there is still time until this will cause "real" problems. Unfortunately, it's a bit urgent anyway, because it makes core unit test fail. This however causes our pipelines to fail which is really annoying when trying to keep the overview of where we have problems and where not. So I would be very happy to see this fixed before PHP 9 :)

gjb2048 commented 7 months ago

@PhMemmel How urgent? Enough for a financial incentive? I mean I will get around to it at some point for free but clearly there are motivations in life to get things done faster.

PhMemmel commented 7 months ago

Thanks again for the quick answer. Unfortunately I cannot provide a financial incentive. I instead provided a PR, I hope it's nearly as good as the incentive ;-)

gjb2048 commented 7 months ago

Dear @PhMemmel,

Thanks for the patch, I've reviewed and found https://github.com/gjbarnard/moodle-format_topcoll/pull/148/commits/dde0f25cad2796e152e4126337701b0c17be2c98#r1506277983 - I'll need to spend time re-understanding my code in order to know if the rest is correct.

Kind regards,

Gareth

PhMemmel commented 7 months ago

Thank you very much for having a look that quick, I fixed what you found and expect your feedback on the rest of the code!

gjb2048 commented 7 months ago

@PhMemmel No further comments :) - hope to do a release soon. Merged into main and squashed https://github.com/gjbarnard/moodle-format_topcoll/commit/c95f2b346fe078397cae8d18aa1658fd30f9018b. Also will almost certainly backport to keep the method calls the same and prevent future conflicts.