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

Rename http3Only to requireUnreliable #1745

Merged
merged 3 commits into from
May 2, 2024
Merged

Rename http3Only to requireUnreliable #1745

merged 3 commits into from
May 2, 2024

Conversation

annevk
Copy link
Member

@annevk annevk commented Apr 5, 2024

This should be more future proof.

Fixes #1744.


Preview | Diff

This should be more future proof.

Fixes #1744.
fetch.bs Outdated Show resolved Hide resolved
fetch.bs Outdated Show resolved Hide resolved
@jasnell
Copy link

jasnell commented Apr 6, 2024

Given this work-in-progress "Quic for Streams" draft https://www.ietf.org/archive/id/draft-kazuho-quic-quic-on-streams-00.html ... which would allow implementing a functional subset of QUIC on top of TLS, I'm not sure "requireUnreliable" makes sense

@annevk
Copy link
Member Author

annevk commented Apr 8, 2024

@jasnell presumably a WebTransport user that sets requireUnreliable to true would not want such a connection though? Maybe it does mean we should explicitly say HTTP/3 over UDP when we give examples?

@ronag
Copy link
Contributor

ronag commented Apr 8, 2024

What does requireUnreliable even mean in a practical sense? Why would I ever want to require that something is "unreliable"? It just sounds weird... enableUnreliable, allowUnreliable etc. maybe is less confusing?

@annevk
Copy link
Member Author

annevk commented Apr 8, 2024

That's the term https://w3c.github.io/webtransport/ uses as public API and it's for them, so might as well stick with it. What it means is described in this PR.

Copy link

@ekinnear ekinnear left a comment

Choose a reason for hiding this comment

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

Thank you! This seems like it strikes a good balance between describing the needs at a high level and being clear and precise enough for implementation at a lower level.

fetch.bs Outdated Show resolved Hide resolved
@annevk
Copy link
Member Author

annevk commented Apr 24, 2024

Thanks! I've pushed mt's suggestion. I will merge this on Monday barring any other comments.

fetch.bs Show resolved Hide resolved
@annevk
Copy link
Member Author

annevk commented Apr 29, 2024

Thanks, given there's been feedback that resulted in further changes I will merge it this week Thursday now to give everyone a chance to look again.

@annevk annevk merged commit 14bad1d into main May 2, 2024
2 checks passed
@annevk annevk deleted the annevk/http3Only branch May 2, 2024 08:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Consider renaming or replacing http3only?
6 participants