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

Switch async code to use futures crate #171

Closed
wants to merge 1 commit into from
Closed

Switch async code to use futures crate #171

wants to merge 1 commit into from

Conversation

no111u3
Copy link
Contributor

@no111u3 no111u3 commented Nov 26, 2019

Remove using nb crate for asynchronously run code - use core::futures for replace it.
RFC for these changes is #172

@rust-highfive
Copy link

r? @thejpster

(rust_highfive has picked a reviewer for you, use r? to override)

Copy link
Contributor

@thejpster thejpster left a comment

Choose a reason for hiding this comment

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

I like the idea, but it's a significant change so we should probably RFC it, or at least discuss it in the meeting.

We shall need to fix the build failures.

@thejpster
Copy link
Contributor

Is there a reason this is using futures 0.1 instead of 0.3?

@no111u3
Copy link
Contributor Author

no111u3 commented Nov 26, 2019

@thejpster I fixed build on last commit. It is clean compatible changes with older code - only remove nb but need to switch minor version (I don't create RFC because it's may be applied anytime, It's try to make code clean). For reason of prefer 0.1 - it was my try to reproduce old PR #13, now I change it to core implementation without external crates. Thanks.

@no111u3 no111u3 requested a review from thejpster November 26, 2019 18:01
@korken89
Copy link

Interesting, I had no idea Poll had been stabilized.
We could probably replace the nb crate now.

@therealprof therealprof added needs-decision S-blocked Status: marked as blocked ❌ on something else such as an RFC or other implementation work. labels Nov 26, 2019
@therealprof
Copy link
Contributor

As per 11-26 meeting this will require an RFC to move ahead.

@Nemo157
Copy link

Nemo157 commented Nov 27, 2019

Moved to RFC

@no111u3
Copy link
Contributor Author

no111u3 commented Nov 27, 2019

@therealprof I added RFC on head message of this thread.

@no111u3
Copy link
Contributor Author

no111u3 commented Nov 27, 2019

@Nemo157 I understand that problem we have but in current implementation of api i couldn't change behavior of run without more breakable changes on api. I'm planning to find solution in this situation. Thanks.

@no111u3
Copy link
Contributor Author

no111u3 commented Nov 29, 2019

Need to review of architecture solution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-decision S-blocked Status: marked as blocked ❌ on something else such as an RFC or other implementation work. S-waiting-on-review Review is incomplete T-hal
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants