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.

Sign up for GitHub

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

Jump to bottom

Added more text to NCID section #3535

Merged
merged 2 commits into from
Mar 23, 2020
Merged

Added more text to NCID section #3535

merged 2 commits into from
Mar 23, 2020

Conversation

martinduke
Copy link
Contributor

Fixes #3534

erickinnear
erickinnear reviewed Mar 19, 2020
View reviewed changes
draft-ietf-quic-transport.md Outdated Show resolved Hide resolved
draft-ietf-quic-transport.md Outdated Show resolved Hide resolved
martinthomson
martinthomson reviewed Mar 20, 2020
View reviewed changes
draft-ietf-quic-transport.md Outdated Show resolved Hide resolved
draft-ietf-quic-transport.md Outdated Show resolved Hide resolved
@martinthomson martinthomson added -transport design An issue that affects the design of the protocol; resolution requires consensus. labels Mar 20, 2020
@martinthomson
Copy link
Member

If you drop the first paragraph, this is just an editorial fix as far as I'm concerned.

That is, not sending a frame that was previously sent is not a change. And - assuming that you dump it - "immediately" has no defined meaning anyway.

@martinduke
Copy link
Contributor Author

@martinthomson I'm happy to drop the first paragraph, and I'm glad to hear that it's editorial. Two people on our team took the existing text to mean that we had to send RCID after receiving NCID even though we'd previously retired that sequence number.

Update coming soon.

@martinduke martinduke changed the title Added more test to NCID section Added more text to NCID section Mar 20, 2020
@martinduke
Copy link
Contributor Author

@martinthomson Feel free to change to "editorial"

nibanks
nibanks approved these changes Mar 20, 2020
View reviewed changes
Copy link
Member

@nibanks nibanks left a comment

Choose a reason for hiding this comment

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

Looks fine to me, though I do think it's better with some kind of language to send it sooner rather than later.

draft-ietf-quic-transport.md
NEW_CONNECTION_ID frame MUST immediately send a corresponding
RETIRE_CONNECTION_ID frame that retires the newly received connection ID.

NEW_CONNECTION_ID frame MUST send a corresponding RETIRE_CONNECTION_ID frame
Copy link
Member

Choose a reason for hiding this comment

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

"promptly send"? Though that's almost the same as immediately.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, this is fine as is.

@martinthomson martinthomson added editorial An issue that does not affect the design of the protocol; does not require consensus. and removed design An issue that affects the design of the protocol; resolution requires consensus. labels Mar 22, 2020
@martinthomson martinthomson merged commit e7715a9 into quicwg:master Mar 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@erickinnear erickinnear erickinnear left review comments

@martinthomson martinthomson martinthomson left review comments

@nibanks nibanks nibanks approved these changes

@ianswett ianswett ianswett approved these changes

Assignees
No one assigned
Labels
-transport editorial An issue that does not affect the design of the protocol; does not require consensus.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

NEW_CONNECTION_ID text is too restrictive on receivers
5 participants
@martinduke @martinthomson @ianswett @nibanks @erickinnear