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

RFC6265bis: Advise the reader which section to implement #2478

Merged
merged 7 commits into from
Mar 21, 2023

Conversation

sbingler
Copy link
Collaborator

Closes #2289

As mentioned in #2289, I've seen a few instances where an implementer chose the wrong section for their needs which caused compatibility issues down the line.

This PR adds a section that will (hopefully) grab the reader's attention and direct them to the correct section.

@sbingler
Copy link
Collaborator Author

@miketaylr PTAL

Copy link
Collaborator

@miketaylr miketaylr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, just a few editorial comments / suggestions / questions.

draft-ietf-httpbis-rfc6265bis.md Outdated Show resolved Hide resolved
draft-ietf-httpbis-rfc6265bis.md Outdated Show resolved Hide resolved
draft-ietf-httpbis-rfc6265bis.md Outdated Show resolved Hide resolved
draft-ietf-httpbis-rfc6265bis.md Outdated Show resolved Hide resolved
draft-ietf-httpbis-rfc6265bis.md Outdated Show resolved Hide resolved
draft-ietf-httpbis-rfc6265bis.md Outdated Show resolved Hide resolved
draft-ietf-httpbis-rfc6265bis.md Outdated Show resolved Hide resolved
@sbingler
Copy link
Collaborator Author

@mikewest
PTAL

Copy link
Collaborator

@miketaylr miketaylr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, much clearer - thanks.

draft-ietf-httpbis-rfc6265bis.md Outdated Show resolved Hide resolved
Copy link
Member

@mikewest mikewest left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This advice makes sense to me, LGTM.

But I have a suggestion around structure that I'd appreciate you considering before landing it. WDYT?

@@ -460,6 +460,77 @@ Set-Cookie: lang=; Expires=Sun, 06 Nov 1994 08:49:37 GMT
Cookie: SID=31d4d96e407aad42
~~~

## Which Requirements to Implement
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Optional Structural Nit: This comes right after the "Examples" section, and right before two very long sections of requirements for servers on the one hand and user agents on the other. It might be helpful to provide that context here. "Hey, you're going to read two long sections in a minute. Based on who you are, you might want to pay more attention to this one."

Alternatively, I could imagine a note at the top of both the requirements section pointing off to this section as a subsection of "Implementation Considerations". That might be clearer.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reworked the opening paragraph, wdyt?

Copy link
Member

@mikewest mikewest left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@mikewest mikewest merged commit 6979693 into httpwg:main Mar 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

RFC 6265bis: The spec should more clearly advise which parts a reader should implement
3 participants