Block module is the only other module to call CacheDecorator with all of the arguments.
Patch for review: should it use CACHE_PERMANENT?
Expanded the issue summary.
This bug keeps D8 from being installed on postgreSQL and SQLite and therefore is critical.
While there are additional bugs to be fixed (see #2001350: [meta] Drupal cannot be installed on PostgreSQL resp. #1998366: [meta] SQLite is broken), this is a confirmed and obvious bug and the fix allows the installer to get a bit further.
CACHE_PERMANENT is the default value for expiration, so unless otherwise set it should be just fine.
Just putting the USE statements into alphabetic order and breaking out two of the arguments to intermediate variables to avoid the horribly long line.
So please do another quick review, otherwise this patch should be RTBC, IMHO. Would be nice to go forward then.
Wow, nice catch!
I'm not 100% sure we can/should write tests for this.
I'll tentatively set RTBC, but if someone wants tests they should be prepared to suggest how to best do that :)
Expanded and partially reworded
I assume if we had postgres test bots they'd have caught this via existing test coverage (or just not installing at all). Can't think of a test otherwise.
I agree this isn't easily testable. Committed to 8.x. Thanks.
Thanks for committing, and yes, postgres test bots would have definitely caught this.
Checking the type of parameters on all API functions might help, too, but would add an incredible overhead given that PHP is a weakly typed language. Otherwise we unfortunatly can't completely avoid occasional wrong function calls when code is refactored.
Automatically closed -- issue fixed for 2 weeks with no activity.
Add related issue #1903346: Establish a new DefaultPluginManager to encapsulate best practices, and update summary.
Drupal is a registered trademark of Dries Buytaert.