search

themehybrid / hybrid-core

Official repository for the Hybrid Core WordPress development framework.
GNU General Public License v2.0
689 stars 144 forks source link

4.0: ext/get-the-image.php needs PHP 7.3 compatibility upgrade for `compact()` #173

Open lkraav opened 5 years ago

lkraav commented 5 years ago

compact() behavior was changed in PHP 7.3 and get-the-image.php usage of it should probably be rearchitected https://github.com/justintadlock/hybrid-core/blob/4.0.0/ext/get-the-image.php#L298

If left unattended, it's likely that people's error logs will be flooded with Notice | compact(): Undefined variable: post_id | 51 | wp-content/themes/hybrid-core-4.0/ext/get-the-image.php:298.

For example on a category page for me this notice trigger 1800 times per page load (many featured images).

https://www.php.net/manual/en/function.compact.php

EDIT https://core.trac.wordpress.org/ticket/44416

justintadlock commented 5 years ago

I'd recommend switching to Hybrid Carbon, even if on 4.0 of the framework: https://github.com/justintadlock/hybrid-carbon

lkraav commented 5 years ago

Thanks. That's a good idea.

While we get ourselves migrated, can you drop a hint on what could be a good strategy to optimize compact() out of $key = md5( serialize( compact( array_keys( $this->args ) ) ) ); caching?

I'm not entirely sure why it's there in the first place, why not just serialize $this->args directly?