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

Support smol runtime #1719

Merged
merged 3 commits into from
Jan 24, 2024
Merged

Support smol runtime #1719

merged 3 commits into from
Jan 24, 2024

Conversation

al8n
Copy link
Contributor

@al8n al8n commented Dec 6, 2023

Support smol async runtime.

djc
djc reviewed Dec 6, 2023
View reviewed changes
quinn/src/runtime.rs Outdated Show resolved Hide resolved
Ralith
Ralith reviewed Dec 7, 2023
View reviewed changes
quinn/src/runtime.rs Outdated Show resolved Hide resolved
@al8n al8n requested a review from djc December 8, 2023 10:19
djc
djc reviewed Dec 8, 2023
View reviewed changes
Copy link
Member

@djc djc left a comment

Choose a reason for hiding this comment

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

Okay, please turn this into three commits:

  • Introduce UdpSocket::new() and use it in the AsyncStdRuntime impl
  • Wrap AsyncStdRuntime in an inline module, in place
  • Copy the AsyncStdRuntime module to create SmolRuntime

Please minimize unnecessary changes.

quinn/src/runtime/async_io.rs Outdated Show resolved Hide resolved
quinn/src/runtime/async_io.rs Outdated Show resolved Hide resolved
quinn/src/runtime/async_io.rs Show resolved Hide resolved
@al8n al8n requested a review from djc December 11, 2023 12:43
djc
djc approved these changes Dec 11, 2023
View reviewed changes
Copy link
Member

@djc djc left a comment

Choose a reason for hiding this comment

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

Looking good, just two more nits!

fuzz/slow-unit-65bb0a5939cd99222f9c1767688dbde23edce018 Outdated Show resolved Hide resolved
quinn/src/runtime/async_io.rs Show resolved Hide resolved
@djc
Copy link
Member

djc commented Jan 22, 2024

@al8n are you able to address the remaining issues here? Would be nice to get this merged.

@al8n
Copy link
Contributor Author

al8n commented Jan 22, 2024

@al8n are you able to address the remaining issues here? Would be nice to get this merged.

Ah, yes, I totally forget this PR. I will address the issues tomorrow.

@djc
Copy link
Member

djc commented Jan 23, 2024

(Please rebase to keep the nice commit history instead of adding merges.)

@al8n al8n requested a review from djc January 23, 2024 12:57
@djc djc requested a review from Ralith January 23, 2024 13:28
@al8n al8n force-pushed the main branch 2 times, most recently from b722657 to 71d9dc4 Compare January 23, 2024 16:41
Ralith
Ralith reviewed Jan 23, 2024
View reviewed changes
quinn/src/runtime.rs Outdated Show resolved Hide resolved
@al8n al8n requested a review from Ralith January 24, 2024 04:59
@al8n al8n requested a review from Ralith January 24, 2024 13:27
@djc
Copy link
Member

djc commented Jan 24, 2024

Can you please squash this last commit into the previous one? Other than that, this looks good!

@al8n
Copy link
Contributor Author

al8n commented Jan 24, 2024

Can you please squash this last commit into the previous one? Other than that, this looks good!

Done!

Ralith
Ralith approved these changes Jan 24, 2024
View reviewed changes
Copy link
Collaborator

@Ralith Ralith left a comment

Choose a reason for hiding this comment

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

Thanks!

@djc djc enabled auto-merge (rebase) January 24, 2024 19:13
@djc
Copy link
Member

djc commented Jan 24, 2024

Looks like this needs to be rebased? Not sure why.

@djc djc merged commit 2e4074a into quinn-rs:main Jan 24, 2024
8 checks passed
@Ralith
Copy link
Collaborator

Ralith commented Jan 24, 2024

It was blocked on #1719 (comment) being marked resolved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Ralith Ralith Ralith approved these changes

@djc djc Awaiting requested review from djc djc is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@al8n @djc @Ralith