User tests: Successful: Unsuccessful:
Code review
The min()
function in PHP returns a mixed
and not int
Status | New | ⇒ | Pending |
Category | ⇒ | Libraries |
well that your call. I don't see how following thePHP documentation is a b.c break though.
I have tested this item
I looked at the code and noted that getMaxUploadSize is used in three places and as this is just a correction to the description of what is returned I think it is OK. If it were to be cast as an int that would be a change that might have b/c consequences. I declare myself a php amateur here so agree it is for the experts to call.
I looked at the code and noted that getMaxUploadSize is used in three places
In Joomla core. You do not know how many calls this method gets (if any) from 3rd Party code in plugins/extensions (probably none, but that's not the point.
Before this PR this method ALWAYS returned with a type of (theoretically) mixed
as that is literally the PHP documented type returned from min()
according to https://www.php.net/manual/en/function.min.php
min() returns the parameter value considered "lowest" according to standard comparisons. If multiple values of different types evaluate as equal (e.g. 0 and 'abc') the first provided to the function will be returned.
If an empty array is passed, then false will be returned and an E_WARNING error will be emitted.
Therefore, to now cast the return to an int
is (theoretically) a backward incompatible change.
Changing the docblock to factually state that the return type from the min is mixed
is fully backward compatible.
@PhilETaylor on your last comment I think that is all what I said or meant. You are working too hard. But keep going!
My last part was disagreeing with @HLeithner
Status | Pending | ⇒ | Fixed in Code Base |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2021-04-05 18:15:10 |
Closed_By | ⇒ | HLeithner | |
Labels |
Added:
?
|
In the end it's like this several years so better documentation follows code then the way around (at least in this case)
Don't get me started with how bad some of the docblocks are in joomla 3 haha
Not sure about this on, I mean it's a b/c break changing this, so maybe cast to int would be better and possible?