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
Nick/aug5reviews #74
Nick/aug5reviews #74
Conversation
@grittygrease I think #73 should land first, and then we should rebase this on top of those changes. Feel free to ignore #75, too, if we think the best outcome here is to simply clarify inline. |
Remove extra "the" Co-authored-by: Christopher Wood <caw@heapingbits.net>
Remove extra "the"
Clarify the terms "initial connection" vs "underlying connection"
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.
Please not the change on line 493.
column with the value "CH, EE, CR" and adding this document in the "Reference" column. | ||
|
||
The addition of "CR" to this column does not authorize the use of this extension |
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.
@grittygrease I think we split the difference between adding a column and just having the text in the I-D but making sure the text is in the registry by asking IANA to add a note with text like:
IANA is also requested to add the following note to the registry:
The addition of the "CR" to the "TLS 1.3" column for the server_name(0) extension does not ...
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 think this is shorter and more clear, ignore if you disagree:
The addition of the "CR" to the "TLS 1.3" column for the server_name(0) extension only marks the extension as valid in a ClientCertificateRequest created as part of client-generated authenticator requests.
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.
lgtm
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.
Text updated. Let me know if this matches what you expect.
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.
lgtm
@grittygrease unless you hear otherwise merge this after 29 October 2021 |
column with the value "CH, EE, CR" and adding this document in the "Reference" column. | ||
|
||
The addition of "CR" to this column does not authorize the use of this extension |
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.
lgtm
No description provided.