#56041 site health full page cache#2894
Conversation
Clorith
left a comment
Clorith
left a comment
There was a problem hiding this comment.
I had a look over here, and had some feedback, and questions about approaches that I've left in-line.
|
@Clorith @swissspidy @spacedmonkey feedback applied.
|
felixarntz
left a comment
felixarntz
left a comment
There was a problem hiding this comment.
@manuelRod This looks solid to me. Other than the existing feedback, I mostly have some nit-pick comments, which are worth addressing to polish the implementation.
felixarntz
left a comment
felixarntz
left a comment
There was a problem hiding this comment.
@manuelRod Just some final nit-picks here, based on the previous comments.
src/wp-includes/rest-api/endpoints/class-wp-rest-site-health-controller.php
Outdated
Show resolved
Hide resolved
|
@felixarntz thanks for the feedback. I've replaces all "page caching" appearances with "page cache". |
# Conflicts: # src/wp-admin/includes/class-wp-site-health.php # tests/phpunit/tests/site-health.php
|
@felixarntz I've updated the PR with trunk and solved the conflicts. All good! |
felixarntz
left a comment
felixarntz
left a comment
There was a problem hiding this comment.
Excellent work @manuelRod!
|
Committed in https://core.trac.wordpress.org/changeset/54043. |
6 participants
Introducing Full Page Cache check into Site Health
Trac ticket: #56041
This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.