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

Callback constructor for FutureLink #1272

Merged
merged 4 commits into from
Jun 21, 2020

Conversation

kellpossible
Copy link
Contributor

Description

This is an initial implementation, just requesting comments at this stage. I still need to implement some examples.

  • Add a new method FutureLink::callback_future(), and implement it for ComponentLink and AgentLink.
  • Refactor the FutureLink::send_future() to take Into<Message> instead of Message to make it match ComponentLink::send_message() more closely, and more convenient to use.

Implements #1271

Checklist:

  • I have ran ./ci/run_stable_checks.sh
  • I have performed a self-review of my own code
  • I have added tests that prove my fix is effective or that my feature works

yewtil/src/future.rs Outdated Show resolved Hide resolved
yewtil/src/future.rs Outdated Show resolved Hide resolved
@kellpossible
Copy link
Contributor Author

kellpossible commented May 30, 2020

I'll also update the documentation for ComponentLink::callback to reference this feature (if the PR for the docs on the other ComponentLink methods gets approved).

@jstarry
Copy link
Member

jstarry commented Jun 8, 2020

@kellpossible these changes look solid to me!

@jstarry
Copy link
Member

jstarry commented Jun 20, 2020

@kellpossible cool if I merge this?

@kellpossible
Copy link
Contributor Author

I never added an example, do you think it's okay?

@jstarry
Copy link
Member

jstarry commented Jun 20, 2020

Yeah, that's fine with me

@kellpossible
Copy link
Contributor Author

kellpossible commented Jun 21, 2020

It also lacks tests. But if its good to merge then go ahead. I also ran the tests I believe but didn't check the box because I hadn't made the example.

@jstarry jstarry marked this pull request as ready for review June 21, 2020 09:03
@jstarry jstarry merged commit 125812d into yewstack:master Jun 21, 2020
@kellpossible
Copy link
Contributor Author

Great, thanks @jstarry !

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