For some reason I can't reproduce the notice... could you? I assume it is fine to merge. Not sure where/if we use
$period . '' or something similar in any meaningful context besides debug logs.
It's reproducible in tests, I didn't try to reproduce through the UI, I think there must have been some error that occurs w/ a week or month period that might trigger it. Ie, something that actually triggers the
some failing tests but otherwise good to merge 👍
@tsteur had to make a bunch of changes to the range period test and noticed that it might be possible for a plugin to use
Period::toString() and expect arrays of arrays. Think this might be a BC issue?
I don't think it is a BC concern but hard to say. But thinking about it again... a
toString method should really not return arrays. Not sure how it is used... but ideally it would actually return a string and not arrays.
I think currently __toString() returns a string, and toString() returns an array of strings. Could try and change it to return a string, will have to see how much code uses it.
All good, don't worry about it right now 👍
Feel free to merge if it fixes the problem. Only usage I could find was SearchEngine feature that used the day period and it doesn't seem to regress there.