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

Take a stab at refactoring the use of Note:. #258

Merged
merged 3 commits into from May 16, 2023
Merged

Take a stab at refactoring the use of Note:. #258

merged 3 commits into from May 16, 2023

Conversation

lnicco
Copy link
Collaborator

@lnicco lnicco commented Oct 24, 2022

This is just a start. While these are editorial changes they will
definitely require discussion for how it's best to change each Note:.

This is just a start. While these are editorial changes they will
definitely require discussion for how it's best to change each Note:.
draft-ietf-quic-qlog-h3-events.md Outdated Show resolved Hide resolved
Comment on lines 227 to 229
This event can contain any number of unspecified fields. This is to
allow representation of unknown settings like grease settings or parameters of
non-standard extensions.
Copy link
Member

Choose a reason for hiding this comment

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

I think this is what we're trying to say?

Suggested change
This event can contain any number of unspecified fields. This is to
allow representation of unknown settings like grease settings or parameters of
non-standard extensions.
This event can contain any number of unspecified fields. This
allows for representation of reserved settings (aka grease) or ad-hoc support for extension settings
that do not have a related qlog schema definition .

@@ -247,7 +246,7 @@ HTTPParametersRestored = {
~~~
{: #http-parametersrestored-def title="HTTPParametersRestored definition"}

Note that, like for parameters_set above, this event can contain any number of
Similarly to HTTPParametersSet this event can contain any number of
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Similarly to HTTPParametersSet this event can contain any number of
Similar to HTTPParametersSet, this event can contain any number of

@@ -247,7 +246,7 @@ HTTPParametersRestored = {
~~~
{: #http-parametersrestored-def title="HTTPParametersRestored definition"}

Note that, like for parameters_set above, this event can contain any number of
Similarly to HTTPParametersSet this event can contain any number of
unspecified fields to allow for additional and custom settings.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
unspecified fields to allow for additional and custom settings.
unspecified fields to allow for reserved or extension settings.

Comment on lines 299 to 300
This event is logged at the time the frame header is created. A frame payload
may require multiple write operations that are logged using data_moved events.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
This event is logged at the time the frame header is created. A frame payload
may require multiple write operations that are logged using data_moved events.
might extend over multiple write operations, which would be logged using multiple data_moved events.

draft-ietf-quic-qlog-h3-events.md Outdated Show resolved Hide resolved
Copy link
Member

@LPardue LPardue left a comment

Choose a reason for hiding this comment

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

LGTM!

@lnicco lnicco marked this pull request as ready for review May 16, 2023 17:47
@lnicco lnicco merged commit 9dc6d24 into main May 16, 2023
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants