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

Make private traits in public api, sealed and public #3127

Merged
merged 2 commits into from Jan 31, 2023

Conversation

programatik29
Copy link
Contributor

Resolves #2051, resolves #3097

  • Moved, sealed, renamed ConnStreamExec to Http2ConnExec and made it public.
  • Changed pub(super) restriction of HttpService to pub.

@seanmonstar
Copy link
Member

Thanks so much! I do think this is the direction that solves all the problems listed in #3097.

What do others think? @sfackler, @LucioFranco

@programatik29
Copy link
Contributor Author

I think documentation can be better. Where do you think it should be @seanmonstar ?

Some ideas:

  • serve_connection method.
  • hyper::server::conn::http2 module.
  • Executor trait.
  • hyper::rt module.
  • hyper::rt::bounds module.
  • Http2ConnExec trait.

@programatik29
Copy link
Contributor Author

In my opinion, there should be a documentation of how should an executor be in Executor trait and link to that trait in related places. Also explain in Http2ConnExec docs that it can be used with any executor implemented like it is shown in Executor trait docs.

@seanmonstar
Copy link
Member

Good point, more docs would help people. Here's some thoughts of what to put where (anyone is free to suggest differently!):

  • http2::Builder::new - Can explain briefly what the argument is, linking to the Http2ConnExec trait (or bounds module, perhaps).
  • serve_connection - Briefly explain, like above.
  • Http2ConnExec - Explain that this trait is for bounds, and that any desire to implement the trait should be done by implementing an Executor, linking to it.
  • bounds - Explain that the module contains traits that ease setting bounds, but that traits within that module likely can be implemented automatically by implementing a different trait. I can't say bounds will only ever be for Executor, so that's what I'm slightly generic here. But there can be a subheading for the bounds docs about Executor.

@seanmonstar seanmonstar merged commit fc9f307 into hyperium:master Jan 31, 2023
@programatik29 programatik29 deleted the sealed-h2-executor branch April 27, 2023 22:02
0xE282B0 pushed a commit to 0xE282B0/hyper that referenced this pull request Jan 12, 2024
…unds` (hyperium#3127)

Define public trait aliases that are sealed, but can be named externally, and have documentation showing how to provide a Executor to match the bounds, and how to express the bounds in your own API.

Closes hyperium#2051
Closes hyperium#3097
0xE282B0 pushed a commit to 0xE282B0/hyper that referenced this pull request Jan 16, 2024
…unds` (hyperium#3127)

Define public trait aliases that are sealed, but can be named externally, and have documentation showing how to provide a Executor to match the bounds, and how to express the bounds in your own API.

Closes hyperium#2051
Closes hyperium#3097

Signed-off-by: Sven Pfennig <s.pfennig@reply.de>
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.

Nameable Future-Proof hyper Executors Don't use private traits in public APIs
2 participants