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

Consolidate quic trait redundancy #173

Merged
merged 2 commits into from
Jun 1, 2024

Conversation

FlorianUekermann
Copy link
Contributor

@FlorianUekermann FlorianUekermann commented Mar 10, 2023

The OpenStreams and SendStream quic traits overlap significantly. This has two effects if both are implemented on the same type:

  1. Unnecessary repetition of identical implementations
  2. Method ambiguity if both traits are imported, which makes calling redundant methods very awkward (need to specify trait, even though the methods actually do the same thing).

A convenient side effect is that this allows using different types for open errors and accept errors. (Most importantly closed quic connections typically return Err(_) on the poll_open methods, while returning Ok(None) on the poll_accept methods) and therefore include fewer variants.

Copy link
Contributor

@Ruben2424 Ruben2424 left a comment

Choose a reason for hiding this comment

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

Thanks for the PR!
I really like this change.
Since this is changing the traits I would like to get a +1 from a second collaborator before merging.

@Ruben2424 Ruben2424 merged commit 793bd17 into hyperium:master Jun 1, 2024
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants