Skip to content
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

Genart last call review of draft-ietf-httpbis-cache-16 #847

Closed
reschke opened this issue May 31, 2021 · 1 comment
Closed

Genart last call review of draft-ietf-httpbis-cache-16 #847

reschke opened this issue May 31, 2021 · 1 comment

Comments

@reschke
Copy link
Contributor

reschke commented May 31, 2021

From https://lists.w3.org/Archives/Public/ietf-http-wg/2021AprJun/0157.html:

Reviewer: Mohit Sethi
Review result: Ready with Nits

I am the assigned Gen-ART reviewer for this draft. The General Area
Review Team (Gen-ART) reviews all IETF documents being processed
by the IESG for the IETF Chair. Please treat these comments just
like any other last call comments.

For more information, please see the FAQ at

https://trac.ietf.org/trac/gen/wiki/GenArtfaq.

Document: draft-ietf-httpbis-cache-16
Reviewer: Mohit Sethi
Review Date: 2021-05-30
IETF LC End Date: 2021-06-10
IESG Telechat date: 2021-06-17

Summary: This draft specification defines HTTP caches and header fields for
controlling the cache behavior.

Major issues:

Minor issues:

  • In the HTML version of the draft, the reference to [Semantics] does not work
    properly. I looked at the xml source which looks fine. I suspect it is
    something to do with the tooling.

  • It is not clear to me which draft is creating the "Hypertext Transfer
    Protocol (HTTP) Field Name Registry". It seems both this draft and
    draft-ietf-httpbis-semantics are creating it? Perhaps you could remove the text
    in this draft saying "introduce the new" and just ask IANA to update the
    registry with fields in Table 1 of this draft.

Nits/editorial comments:

  • Section 1: When does a client or server act as "tunnel"? I don't know if it
    is absolutely necessary to explain the term. You can decide.

  • Section 1: HTTP caching's goal is significantly improving performance -> HTTP
    caching's goal is to significantly improve performance?

  • Section 1.3: Maybe it is obvious to many readers, but I was not sure what is
    meant by a "canned string"?

  • Section 3 vs Section 4: "A cache MUST NOT store a response to a request
    unless:" does not have a comma before unless while "When presented with a
    request, a cache MUST NOT reuse a stored response, unless:" has a comma before
    unless?

  • Some of the bullets in section 3 and 4 were hard to parse. Take for example:
    "When presented with a request, a cache MUST NOT reuse a stored response,
    unless: the stored response does not contain the no-cache cache directive
    (Section 5.2.2.4), unless it is successfully validated (Section 4.3), and". I
    am not sure how to simplify the text on all these requirements.

@reschke
Copy link
Contributor Author

reschke commented May 31, 2021

From https://lists.w3.org/Archives/Public/ietf-http-wg/2021AprJun/0161.html:

Hi Mohit,

Thanks for the review. Responses below.

On 31 May 2021, at 12:14 am, Mohit Sethi via Datatracker noreply@ietf.org wrote:

  • In the HTML version of the draft, the reference to [Semantics] does not work
    properly. I looked at the xml source which looks fine. I suspect it is
    something to do with the tooling.

Julian has covered this, I think.

  • It is not clear to me which draft is creating the "Hypertext Transfer
    Protocol (HTTP) Field Name Registry". It seems both this draft and
    draft-ietf-httpbis-semantics are creating it? Perhaps you could remove the text
    in this draft saying "introduce the new" and just ask IANA to update the
    registry with fields in Table 1 of this draft.

That text includes a reference to the Semantics text creating the registry, so I think it's clear which is doing the work. It's important to mention this because it's a fairly substantial change from IANA's standpoint.

Nits/editorial comments:

  • Section 1: When does a client or server act as "tunnel"? I don't know if it
    is absolutely necessary to explain the term. You can decide.

This is defined in Semantics; I've added a reference in ee8c9306972

  • Section 1: HTTP caching's goal is significantly improving performance -> HTTP
    caching's goal is to significantly improve performance?

That introduces a split infinitive.

  • Section 1.3: Maybe it is obvious to many readers, but I was not sure what is
    meant by a "canned string"?

Fixed in 51bfc119585

  • Section 3 vs Section 4: "A cache MUST NOT store a response to a request
    unless:" does not have a comma before unless while "When presented with a
    request, a cache MUST NOT reuse a stored response, unless:" has a comma before
    unless?

da887ff5

  • Some of the bullets in section 3 and 4 were hard to parse. Take for example:
    "When presented with a request, a cache MUST NOT reuse a stored response,
    unless: the stored response does not contain the no-cache cache directive
    (Section 5.2.2.4), unless it is successfully validated (Section 4.3), and". I
    am not sure how to simplify the text on all these requirements.

We've struggled with how to clearly state these requirements for some time now; the feedback we've received during and since RFC7234 indicates that this format is preferred. That said, the specific phrasing is undoubtedly not perfect, and we're open to suggestions for improvements.

Cheers and thanks again,

reschke added a commit that referenced this issue May 31, 2021
@reschke reschke closed this as completed May 31, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

1 participant