-
Notifications
You must be signed in to change notification settings - Fork 139
Implement opt-in PERFLAB_PLACE_OBJECT_CACHE_DROPIN for Server-Timing object-cache.php placement
#1996
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Implement opt-in PERFLAB_PLACE_OBJECT_CACHE_DROPIN for Server-Timing object-cache.php placement
#1996
Conversation
…OBJECT_CACHE_DROPIN` constant
…OBJECT_CACHE_DROPIN`
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## trunk #1996 +/- ##
==========================================
- Coverage 72.51% 72.50% -0.02%
==========================================
Files 85 88 +3
Lines 7023 7146 +123
==========================================
+ Hits 5093 5181 +88
- Misses 1930 1965 +35
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
felixarntz
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@b1ink0 Thanks for the PR! Basically looks good to go, just a few nit-picks that would be great to address before merge.
plugins/performance-lab/load.php
Outdated
| global $wp_filesystem; | ||
|
|
||
| // Bail if the drop-in is not enabled. | ||
| if ( ! defined( 'PERFLAB_PLACE_OBJECT_CACHE_DROPIN' ) || ! PERFLAB_PLACE_OBJECT_CACHE_DROPIN ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit-pick: Maybe move this below, either right before or right after the PERFLAB_DISABLE_SERVER_TIMING clause? Since it's more closely related to that check.
It would also be great if you could update the comment above PERFLAB_DISABLE_OBJECT_CACHE_DROPIN, to clarify that it's maintained only for backward compatibility.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done in f492eed
…est setup. Co-authored-by: Felix Arntz <[email protected]>
felixarntz
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @b1ink0!
Summary
Fixes #1633
Relevant technical choices
Introduce
PERFLAB_PLACE_OBJECT_CACHE_DROPINconstant to opt-in to placing the Server-Timingobject-cache.phpdrop-in.