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
Move Generating Acknowledgements to Transport #2916
Merged
Merged
Changes from 1 commit
Commits
Show all changes
22 commits
Select commit
Hold shift + click to select a range
812ced0
Move Generating Acknowledgements to Transport
ianswett f8fc854
Move, don't just remove the text
ianswett cfab7ed
Remove the "Crypto Handshake Data" section
ianswett 4ac701e
Stop referencing QUIC Transport
ianswett 862bcd3
Remove obsolete text
ianswett f364aab
Stop referencing host-delay in Recovery
ianswett 50f2a55
Fix CircleCI (hopefully)
ianswett eed60b2
Update draft-ietf-quic-recovery.md
ianswett 67ece93
Update draft-ietf-quic-transport.md
janaiyengar 6ff4cc0
Update draft-ietf-quic-transport.md
janaiyengar 1a07dcd
Update draft-ietf-quic-transport.md
janaiyengar ce3c950
Update draft-ietf-quic-transport.md
janaiyengar d77285c
Update draft-ietf-quic-transport.md
janaiyengar 19df215
Addresses comments and some text rework
janaiyengar 55f1ad9
re-adding (with some rework) text that was dropped inadvertently
janaiyengar d60f633
rebase cleanup
janaiyengar 17196dc
Update draft-ietf-quic-transport.md
ianswett 02bbcdf
Update draft-ietf-quic-transport.md
ianswett 35a6072
Update draft-ietf-quic-transport.md
ianswett ac3e437
Update draft-ietf-quic-transport.md
ianswett 6910bce
Reflow a line
ianswett 5a8af76
moving text around a bit and mt's comment
janaiyengar File filter
Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
These are both good and useful statements, but they sort of stand awkwardly next to each other as if they are strangers.
I tried to say something about the fact that this also provides some assurances about acknowledgement behaviour that senders could use when innovating with new algorithms, but it ended up being quite hard to do that without distracting too much from the core purpose of the section. Also, the SHOULD got in the way: there is no real assurance, there we have to rely on the fact that this is a good recommendation that most deployments will likely respect.
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.
@ianswett, not planning to do anything about this? I'll merge it now if you think we should just leave this alone.
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 want to do another larger pass over this text, but I'd like to keep it as is for now.
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'm trying out something -- give me a few minutes
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.
Ok, a few changes, look at the last commit. I've moved some text around and I think I've captured the spirit of this more clearly.