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

Deemphasize TCP SACKs reference in ACK frame description #1312

Merged

Conversation

dtikhonov
Copy link
Member

It is odd to describe something by saying what is not like. In
addition, not all readers of the document will be familiar with
TCP SACKs.

It is odd to describe something by saying what is *not* like.  In
addition, not all readers of the document will be familiar with
TCP SACKs.
Unlike TCP SACKs, QUIC acknowledgements are irrevocable. Once a packet has
been acknowledged, even if it does not appear in a future ACK frame,
it remains acknowledged.
QUIC acknowledgements are irrevocable: Once a packet has been acknowledged,
Copy link
Member

Choose a reason for hiding this comment

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

Period, not colon.

it remains acknowledged.
QUIC acknowledgements are irrevocable: Once a packet has been acknowledged,
even if it does not appear in a future ACK frame, it remains acknowledged.
(This is unlike TCP SACKs.)
Copy link
Member

Choose a reason for hiding this comment

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

Don't include an entire sentence in a parenthetical. It would be fine to remove the paretheses here.

Of course, now that you touched this, I notice that we don't have a TCP SACK reference here, and you can't just use an unexpanded acronym like this without both expansion and a reference.

@janaiyengar
Copy link
Contributor

I think this addresses Martin's comments, so I'll merge.

@janaiyengar janaiyengar merged commit c46f52e into quicwg:master Apr 25, 2018
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

4 participants