should not break anything and gets rid of a warning
Please include a description of this change and which issue it fixes. If no issue exists yet please include context and what problem it solves.
This is still WIP but in case someone else picks this up later I wanted to make it visible.
Just realised this can still be worked on. So, maybe wait with the review until I've done all simple fixes I can do for now.
I lazily return
'1' here as
null to me means it is earlier than anything else and it is elsewhere compared to another version string with version_compare(). I was considering
'0' or just
'' as well. Maybe
'0' or even
'0.0.0' is best?
I lazily return '1' here as null to me means it is earlier than anything else and it is elsewhere compared to another version string with version_compare(). I was considering '0' or just '' as well. Maybe '0' or even '0.0.0' is best?
Afaik this was added somewhen in 2018. So every installation done before might not hold the install version. Simply returning a default value doesn't seem good to me. That way you you loose the information somewhere else if it was recorded or not. But actually I don't have an overview for what that is currently used. Maybe @tsteur knows more.
@geekdenz sorry I don't quite understand what you're meaning? Which
version_compare are you referring to? Generally for
DbHelper::getInstallVersion() we still want to return null when we don't know when it was installed. That can be important to know compared to assuming version 1. Which would be for example shown in the system check potentially etc and is wrong information.
@tsteur Sorry, I could have made this more clear:
is used. I didn't test (which I should have) what it would currently return if null is passed. You are right, assigning a string is losing information, but it generates a deprecation warning in PHP 8.1 (null passed instead of string) and that seemed like the easiest fix. However, I wasn't sure it would cover all the bases, which is why I raised the question.
👍 got it now. I suppose as the method is not used in any non-core plugin and looking at all the usages we could return
'1'. That way all the
!empty() checks on the returned version won't break and I suppose version_compare still works