Skip to content

Conversation

lorenzulrich
Copy link

As of PHP 8.0, a deprecation log item is thrown when a required parameter follows an optional parameter:

Deprecated: Required parameter $totalcount follows optional parameter $courseid in /var/www/html/vendor/moodle/moodle/mod/hsuforum/lib.php on line 2159

By changing the order appropriately, this can be fixed. It is backwards-compatible to any currently supported PHP version (and even much older ones).

@cameron1729
Copy link

cameron1729 commented Jan 24, 2023

I'm not sure how changing the param order can be considered backwards compatible? Indeed, in this patch you have to change calling code.

I would recommend against changing param order. Instead I think it makes more sense to make $courseid, $limitfrom, and $limitnum required params, as it's not possible that hsuforum_search_posts has ever been called without them. The deprecation notice in PHP 8.1 will even tell you this:

Deprecated: Optional parameter $courseid declared before required parameter $totalcount is implicitly treated as a required parameter

Deprecated: Optional parameter $limitfrom declared before required parameter $totalcount is implicitly treated as a required parameter

Deprecated: Optional parameter $limitnum declared before required parameter $totalcount is implicitly treated as a required parameter

And you can play with older versions of PHP back to at least 4.3 to see that this has always been the case. If you try to call hsuforum_search_posts without providing a value for $courseid, $limitfrom, and $limitnum then you get messages like: Warning: Missing argument 5 for hsuforum_search_posts() until you provide at least 5 arguments.

By making those params required, existing behaviour is preserved, and no calling code needs to be modified.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants