Automattic / VIP-Coding-Standards

PHP_CodeSniffer ruleset to enforce WordPress VIP coding standards.
https://wpvip.com/documentation/how-to-install-php-code-sniffer-for-wordpress-com-vip/
Other
237 stars 40 forks source link

New Sniff: wp_get_post_revisions memory issues #468

Open mikeyarce opened 4 years ago

mikeyarce commented 4 years ago

What problem would the enhancement address for VIP?

wp_get_post_revisions can be problematic because it loads the entire post object for each revision. I've seen this in CLI commands and other places, where you might be looping through a set of posts, and then suddenly have to load the entire post objects for every revision a post has. If those revisions are large and numerous - it quickly fails.

Related trac: https://core.trac.wordpress.org/ticket/34560

Describe the solution you'd like

We should suggest they only get the IDs for the revisions instead of the whole object:

wp_get_post_revisions( $post->ID, array( 'fields' => 'ids' ) );

What code should be reported as a violation?

Anything that doesn't have the fields argument, like:

 wp_get_post_revisions( $post->ID );

What code should not be reported as a violation?

Getting the ids field or other fields instead of the whole object.

wp_get_post_revisions( $post->ID, array( 'fields' => 'ids' ) );
GaryJones commented 4 years ago

Getting the ids field or other fields instead of the whole object.

@mikeyarce Am I right in thinking that the conditions for success (no violation) is:

Any further constraints?

Would this apply to WordPress.com VIP as well?

mikeyarce commented 4 years ago

@GaryJones those conditions are perfect, can't think of any other constraints.

Would this apply to WordPress.com VIP as well?

Yes, it would!