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

remove future type param #1

Merged

Conversation

hawkw
Copy link

@hawkw hawkw commented Nov 23, 2020

No description provided.

@hawkw hawkw mentioned this pull request Nov 23, 2020
2 tasks
@hlb8122
Copy link
Owner

hlb8122 commented Nov 24, 2020

@hawkw oh ok, I thought it was more embedded than that - you can literally just delete the Fut lines with no problem. >_>

You've probs noticed that that we no longer have the same kind of constraints we do on the other combinator ServiceExt methods.

    fn map_ok<F, Response>(self, f: F) -> MapOk<Self, F>
    where
        Self: Sized,
        F: FnOnce(Self::Response) -> Response + Clone,
    fn map_err<F, Error>(self, f: F) -> MapErr<Self, F>
    where
        Self: Sized,
        F: FnOnce(Self::Error) -> Error + Clone,

https://docs.rs/futures/0.3.8/futures/future/trait.TryFutureExt.html#method.and_then
etc

If you ok with this I'll merge this then get docs done.

@hawkw
Copy link
Author

hawkw commented Nov 24, 2020

You've probs noticed that that we no longer have the same kind of constraints we do on the other combinator ServiceExt methods.

IMO, that's fine, and I'd move forward on merging this. The map combinators don't take functions that return a future, so they don't need to be generic over its type to introduce this bound. I'd get a second opinion from @LucioFranco when merging your PR, but I think this is okay.

@hlb8122 hlb8122 merged commit 6deaaa2 into hlb8122:add-andthen-combinator Nov 24, 2020
hlb8122 pushed a commit that referenced this pull request Jan 6, 2021
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