Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Incorporate page state into ETag computation #1722
Incorporate page state into ETag computation #1722
Changes from 28 commits
60740a8
2239317
3c7842b
3c57a1d
6a5e421
cc8e252
72f4e54
8651d05
f66012e
56cbe8e
38d0097
d61a8bc
072140b
4ffb79f
1794cff
3c3a1a3
65dc803
1c0bdac
e562726
6d80095
73e4519
59879f0
df306e1
c9c0dd8
4632350
5b4e791
1073780
9a19113
03d1683
8325cb4
1b3ba00
a71fe76
03ef8c8
73edbdb
55d56d3
63bb613
18e0476
57a0ba6
804d71e
8fe2f8f
1869689
7c0cfe1
1fc20af
b083a29
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
should this be the post type?
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.
Hummm. Then when
WP_Term
is the queried object, would thetype
be the taxonomy name? I think actually this isn't needed because all that is needed here is to uniquely identify the queried object with a hash of this data here. The point of thetype
ofpost
is to differentiate it from atype
ofterm
since it is entirely possible for a post and term to have the same ID.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.
In the case of
$current_template
being aWP_Block_Template
then this here would be:That is expected?
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.
The primary goal here is to factor in the
modified
date if it’s present. Would it be better to have a separate key specifically for this, rather than including the entire object?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.
Similarly, for the
queried_object
, should we account for the entire object, or would just the ID andmodified
date (if available) suffice? I believe themodified
date is only available forWP_Post
objects, so for other types likeWP_Term
orWP_User
, this may not apply.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.
I suppose it doesn't hurt to have the entire template object included here. In particular, by including the
content
it ensures that whenever the template content changes the URL Metrics will become stale.For the
queried_object
on the other hand, I think it would be better to limit the fields to:id
,type
, andlast_modified
. Thelast_modified
would only be applicable to posts. In the cast of a post type archive, theid
could benull
as well, with thetype
then being notpost
,term
, oruser
but rather the slug of the post type itself.