-
Notifications
You must be signed in to change notification settings - Fork 381
Add webtransport-hashes to request, wired down to create a connection #1896
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
base: main
Are you sure you want to change the base?
Conversation
|
Build appears to have stalled on infra unrelated to PR? ("Run pip install bikeshed && bikeshed update") cc @annevk |
b296f9e to
6614713
Compare
annevk
left a comment
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 rebased (which should address the CI issue you encountered) and pushed a commit addressing a number of nits. This looks good to me, but I have a couple of questions that might result in a couple more tweaks.
Note that I had to force push to your branch so you'll have to reset if you want to commit and push again.
|
|
||
| <p class=note>This flag is for exclusive use by HTML's render-blocking mechanism. [[!HTML]] | ||
|
|
||
| <p>A <a for=/>request</a> has an associated <dfn export for=request>webtransport-hash list</dfn> (a |
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.
For this and equivalent terms, should we instead use the camelcase wording? E.g., "WebTransport-hash list"? Also, is this a list or a set because you used set/is not empty below?
| "<code>no</code>"), an optional boolean | ||
| <dfn export for="obtain a connection"><var>requireUnreliable</var></dfn> (default false), and an | ||
| optional <a for=/>webtransport-hash list</a> | ||
| <dfn export for="obtain a connection"><var>webTransportHashes</var></dfn> (default « »): |
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.
Note that I made this a named parameter. I'm pretty sure that's needed as the prior parameter is named. I also fixed the default value.
Add an associated webtransport-hashes (a webtransport-hash list) to request, and wire it down to create a connection. For use by WebTransport and CSP w3c/webappsec-csp#791.
Fixes #1880.
Preview | Diff