Once we depend on Drupal 8.7, we can also finally depend on PHP 7 and use all the great new language features available there.
In this issue, to be committed once we increase the minimum core version to 8.7 (shortly after the release of 8.8, I think?), we should not only only add the PHP requirement explicitly to search_api.info.yml, but we can already go through the module and change some syntax – especially !empty($a['foo']) ? $a['foo'] : $b, which probably occurs a lot.
| Comment | File | Size | Author |
|---|---|---|---|
| #13 | 3044782-13--php_7_requirement.patch | 41.12 KB | drunken monkey |
Comments
Comment #2
drunken monkeyNote to self that this is not the same as
$a['foo'] ?? $b, as??only checks forNULL, not falsey values.Comment #3
drunken monkeyComment #4
stefvanlooveren commentedComment #5
stefvanlooveren commentedComment #6
borisson_That looks like it should do the job, I can't set to needs review because our testbots run by default on 5.6 - this gets a +1 though.
Comment #7
stefvanlooveren commentedComment #8
drunken monkeyPatch applies fine for me when copy-pasting from the browser, but fails if downloaded. Might be WIndows line endings?
In any case, thanks a lot for the patch!
Re-roll with UNIX line endings.
@ Joris: I’m pretty sure the default has been PHP 7 for ages now, and a few months ago I switched over the last of our three test environments to 7 as well.
Comment #9
borisson_Ok, in that case this could be moved to rtbc
Comment #10
drunken monkeyNo, since even though we don’t test against PHP 5 anymore, we should still support it as long as Core supports it – i.e., until 8.6 hits EOL in a few weeks. I’d still wait until then, just to be on the safe side.
Also, adding a
@todocomment so that we remember this in the follow-up. (Moved over from #3023170: Test for compatibility with Drupal 9.)Comment #11
drunken monkeyComment #12
drunken monkeyAnd here is a patch also updating what can be updated.
It’s … extensive. But as long as the test bot agrees, it shouldn’t really break anything (except, potentially, a patch or two).
In any case, Drupal 8.6 is dead, so we can finally commit this.
Comment #13
drunken monkeyHuh. Why wasn’t this marked “Needs work” by the test bot.
Anyways, should be fixed with the attached revision.
Comment #15
drunken monkeyHuh, seems I missed this, it has been lying around now for quite long enough.
Committed.
Thanks again, everyone!