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

docs: Note that most futures from Server run on current runtime #866

Merged
merged 2 commits into from
Jun 19, 2021
Merged

docs: Note that most futures from Server run on current runtime #866

merged 2 commits into from
Jun 19, 2021

Conversation

emesterhazy
Copy link
Contributor

The documentation for most of the warp::Server methods (bind,
try_bind_with_graceful_shutdown, etc) indicate that the futures
they return can be run on any runtime. However, these functions
panic if they are called outside the context of a Tokio 1.x runtime.

This commit makes a minor update to the documentation to note that
these function return futures that run on the /current/ runtime.

Note that try_bind can actually be called outside the context of a
Tokio runtime, so the docstring for that method remains unchanged.

The documentation for most of the warp::Server methods (bind,
try_bind_with_graceful_shutdown, etc) indicate that the futures
they return can be run on any runtime. However, these functions
panic if they are called outside the context of a Tokio 1.x runtime.

This commit makes a minor update to the documentation to note that
these function return futures that run on the /current/ runtime.

Note that try_bind can actually be called outside the context of a
Tokio runtime, so the docstring for that method remains unchanged.
@emesterhazy
Copy link
Contributor Author

I think there's also a typo in the documentation for Peek and Tail:

Peek | Represents that tail part of a request path, returned by the tail() filter.

Shouldn't this say "returned by the peek() filter?

Tail | Represents that tail part of a request path, returned by the tail() filter.

Both of these should probably say "Represents the tail part..." instead of that.

Thoughts? I can make those tweaks as well.

@jxs
Copy link
Collaborator

jxs commented Jun 18, 2021

Hi, and thanks!

Thoughts? I can make those tweaks as well.

yeah, seems to me that Peek should be:

Represents the tail part of a request path, returned by the peek() filter.

and Tail should be:

Represents the tail part of a request path, returned by the tail() filter.

so if you want to go ahead and update these as well that would be great!
Also since you are on it wdyt of making peek() and tail() references links to the matching functions page? (you can see an example on how to do it here)

@emesterhazy
Copy link
Contributor Author

Good idea on the links. I'm not sure when, but at some point the compiler got smarter and all that I needed to do is wrap the functions in brackets. While I was at it I turned the reference to full() into a link as well.

@jxs jxs merged commit b21ce92 into seanmonstar:master Jun 19, 2021
@jxs
Copy link
Collaborator

jxs commented Jun 19, 2021

awesome thanks!

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