Automattic / vip-go-mu-plugins

The development repo for mu-plugins used on the WordPress VIP Platform.
https://docs.wpvip.com/
GNU General Public License v2.0
190 stars 102 forks source link

Deprecate `wpcom_vip_get_page_by_path()` #995

Closed fklein-lu closed 2 years ago

fklein-lu commented 5 years ago

It's time to deprecate wpcom_vip_get_page_by_path(), and adapt the VIP Coding Standards accordingly:

Edit: The same applies to wpcom_vip_get_page_by_title().

Second Edit: The VIP function equivalent for get_page_by_path() does not work with post types besides pages, which is s significant deviation from Core.

indralukmana commented 5 years ago

Hi @fklein-lu

Thank you for raising this. I want to ask about the wpcom_vip_get_page_by_title(). I am seeing the code in WordPress Core for the function https://github.com/WordPress/WordPress/blob/da7a80d67fea29c2badfc538bfc01c8a585f0cbe/wp-includes/post.php#L4711

I don't think the function in the WordPress Core is cached. Am right to assume that you want to raise the use of get_page() in the wpcom_vip_get_page_by_title()? https://github.com/Automattic/vip-go-mu-plugins/blob/da7fcd24b773e1957687c6eb56f0b56ddd402d9e/vip-helpers/vip-caching.php#L171

rebeccahum commented 2 years ago

Reading the backscroll/history, I'm seeing that this got reverted because wpcom_vip_get_page_by_path() accounts for cache stampeding. @Automattic/vip-platform-cantina what are your thoughts? Are we all that concerned about cache stampeding? Based on the second edit, I think it's enough to attempt a deprecation.

rebeccahum commented 2 years ago

Actually, on a closer look, the third parameter is $post_type, so it looks like it can be used for non-pages: https://github.com/Automattic/vip-go-mu-plugins/blob/fdd99cf535e55eeda46d7a11a4cbb1603d8bc666/vip-helpers/vip-caching.php#L194

Closing since usage is too widespread to deprecate.