Skip to content

add future::poll_fn #124

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

Closed
wants to merge 2 commits into from
Closed

Conversation

yoshuawuyts
Copy link
Contributor

This adds futures::poll_fn, which is fairly common and useful when defining custom futures. This seems like a generally useful, and easy addition to the futures submodule. Thanks!

Ref

Signed-off-by: Yoshua Wuyts <[email protected]>
@yoshuawuyts
Copy link
Contributor Author

Ooops, just noticed this is a dupe of #123. @stjepang I kind of like this version better if I'm honest. What do you think?

@yoshuawuyts yoshuawuyts mentioned this pull request Aug 29, 2019
Copy link
Contributor

@kpp kpp left a comment

Choose a reason for hiding this comment

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

By the way why don't you close this PR in favor of #123?

@jamesmunns
Copy link

jamesmunns commented Aug 29, 2019

By the way why don't you close this PR in favor of #123?

Hey @kpp, @yoshuawuyts has requested review from @stjepang on the merits of #123 vs #124. As far as I have seen, @stjepang hasn't had a chance to address these points yet. You're involved with both conversations, so you should have visibility on all open discussions.

Additionally, regarding this:

Is it a Not Invented Here stance? Providing a stable API over hiding unstable futures-rs seems better than copy-pasting every function from futures-rs.

Stjepan has provided context around this decision here: #123 (comment), and Yosh has also given context and reasoning around this decision here: #123 (comment). You may certainly have a different opinion, however reiterating it across multiple threads is unlikely to promote reasonable discussions on the topic.

You definitely seem passionate about this project, and that is a good thing! However the design decisions of this crate lie with @stjepang, and it would be appreciated if you presented differing opinions in a constructive way, and when there have been statements from the maintainers here pointing in a different direction, for that to be accepted.

Additionally, this is a project worked on in the free time of the maintainers, many of whom are still on vacation after RustConf. Comments like #118 (comment) and #118 (comment) in short succession requesting re-reviews are not necessary, and reduce the signal to noise ratio for notifications to the maintainers. Please allow a reasonable amount of time (at least a few days) before repinging. There are not so many open issues and PRs, and they are being worked through as the maintainers have available time.

We definitely appreciate the effort you are putting towards the numerous issues, PRs, and comments in this repo, we just ask that you allow time for design and other decisions to be made, and to collaborate productively with us to improve the project.

Please feel free to reach out to me privately via email (on my profile) if you have any questions or would like to discuss.

@kpp
Copy link
Contributor

kpp commented Aug 29, 2019

What a neat response! Thanks

reduce the signal to noise ratio

Will do.

@yoshuawuyts yoshuawuyts requested a review from a user August 30, 2019 18:33
Signed-off-by: Yoshua Wuyts <[email protected]>
@yoshuawuyts
Copy link
Contributor Author

poll_fn is already part of async-std; going to go ahead and close this.

@yoshuawuyts yoshuawuyts deleted the poll_fn branch September 17, 2019 21:34
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.

3 participants