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

Add content for QUIC #191

Merged
merged 18 commits into from
Oct 7, 2022
Merged

Add content for QUIC #191

merged 18 commits into from
Oct 7, 2022

Conversation

salmad3
Copy link
Member

@salmad3 salmad3 commented Sep 14, 2022

Context

Latest preview

Please view the latest Fleek preview here.

@salmad3 salmad3 marked this pull request as draft September 14, 2022 05:38
@salmad3 salmad3 changed the title Add content for QUIC Add content for QUIC & WebTransport Sep 16, 2022
@salmad3 salmad3 marked this pull request as ready for review September 16, 2022 09:07
@salmad3 salmad3 added the ready for review PR is ready for review label Sep 16, 2022
@mxinden
Copy link
Member

mxinden commented Sep 16, 2022

//CC @elenaf9 given that you own libp2p/rust-libp2p#2883.

content/concepts/transport.md Outdated Show resolved Hide resolved
content/concepts/transport.md Outdated Show resolved Hide resolved
content/concepts/transport.md Outdated Show resolved Hide resolved
content/concepts/transport.md Outdated Show resolved Hide resolved
content/concepts/transport.md Outdated Show resolved Hide resolved
content/concepts/transport.md Outdated Show resolved Hide resolved
content/concepts/transport.md Outdated Show resolved Hide resolved
content/concepts/transport.md Outdated Show resolved Hide resolved
content/concepts/transport.md Outdated Show resolved Hide resolved
content/concepts/transport.md Outdated Show resolved Hide resolved
@salmad3 salmad3 added change request PR requires changes. and removed ready for review PR is ready for review labels Sep 21, 2022
@salmad3 salmad3 added the ready for review PR is ready for review label Sep 21, 2022
@salmad3
Copy link
Member Author

salmad3 commented Sep 21, 2022

Thanks @MarcoPolo! Suggestions and feedback incorporated

Copy link
Contributor

@marten-seemann marten-seemann left a comment

Choose a reason for hiding this comment

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

I did a first pass over the QUIC section. Haven’t touched WebTransport yet.

Maybe we can split this PR and first get QUIC merged, and then tackle WebTransport?

content/concepts/transport.md Outdated Show resolved Hide resolved
content/concepts/transport.md Outdated Show resolved Hide resolved
content/concepts/transport.md Outdated Show resolved Hide resolved
content/concepts/transport.md Outdated Show resolved Hide resolved
content/concepts/transport.md Outdated Show resolved Hide resolved
content/concepts/transport.md Outdated Show resolved Hide resolved
content/concepts/transport.md Outdated Show resolved Hide resolved
content/concepts/transport.md Outdated Show resolved Hide resolved
content/concepts/transport.md Outdated Show resolved Hide resolved
content/concepts/transport.md Outdated Show resolved Hide resolved
@salmad3 salmad3 changed the title Add content for QUIC & WebTransport Add content for QUIC Sep 21, 2022
@salmad3
Copy link
Member Author

salmad3 commented Sep 21, 2022

@marten-seemann thanks! Incorporated the suggestions and PRs now stacked

content/concepts/transport.md Outdated Show resolved Hide resolved
content/concepts/transport.md Outdated Show resolved Hide resolved
content/concepts/transport.md Outdated Show resolved Hide resolved
content/concepts/transport.md Outdated Show resolved Hide resolved
content/concepts/transport.md Outdated Show resolved Hide resolved
content/concepts/transport.md Outdated Show resolved Hide resolved
salmad3 and others added 3 commits September 21, 2022 11:22
Co-authored-by: Elena Frank <elena.frank@protonmail.com>
Co-authored-by: Elena Frank <elena.frank@protonmail.com>
@salmad3
Copy link
Member Author

salmad3 commented Sep 21, 2022

Thanks @elenaf9! Incorporated suggestions

Copy link
Contributor

@marten-seemann marten-seemann left a comment

Choose a reason for hiding this comment

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

I think it would be valuable to add the OSI layer diagram in this PR. We'll probably have to word-smith the text accompanying that diagram a bit, but overall, I like where this PR is going :)

content/concepts/transport.md Outdated Show resolved Hide resolved
content/concepts/transport.md Outdated Show resolved Hide resolved
content/concepts/transport.md Outdated Show resolved Hide resolved
content/concepts/transport.md Outdated Show resolved Hide resolved
content/concepts/transport.md Outdated Show resolved Hide resolved
content/concepts/transport.md Show resolved Hide resolved
content/concepts/transport.md Outdated Show resolved Hide resolved
content/concepts/transport.md Outdated Show resolved Hide resolved
content/concepts/transport.md Outdated Show resolved Hide resolved
content/concepts/transport.md Outdated Show resolved Hide resolved
@salmad3 salmad3 removed the change request PR requires changes. label Sep 24, 2022
Copy link
Contributor

@marten-seemann marten-seemann left a comment

Choose a reason for hiding this comment

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

This is beginning to look really good. A few more comments.

content/concepts/transport.md Outdated Show resolved Hide resolved
content/concepts/transport.md Outdated Show resolved Hide resolved
content/concepts/transport.md Outdated Show resolved Hide resolved
content/concepts/transport.md Outdated Show resolved Hide resolved
content/concepts/transport.md Outdated Show resolved Hide resolved
content/concepts/transport.md Outdated Show resolved Hide resolved
content/concepts/transport.md Outdated Show resolved Hide resolved
content/concepts/transport.md Outdated Show resolved Hide resolved
content/concepts/transport.md Outdated Show resolved Hide resolved
content/concepts/transport.md Outdated Show resolved Hide resolved
Copy link
Contributor

@marten-seemann marten-seemann left a comment

Choose a reason for hiding this comment

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

Looks like there's still 2 unaddressed comments. Did you miss those @DannyS03?

Co-authored-by: Marten Seemann <martenseemann@gmail.com>
@salmad3
Copy link
Member Author

salmad3 commented Oct 4, 2022

@marten-seemann checking if there are any other parts to reconsider

@salmad3 salmad3 removed the ready for review PR is ready for review label Oct 7, 2022
@salmad3 salmad3 merged commit 01313f1 into master Oct 7, 2022
@salmad3 salmad3 deleted the transport/quic branch January 3, 2023 08:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Add QUIC to transport documentation
5 participants