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

Removed futuroscope #133

Merged
merged 1 commit into from
Jan 6, 2018
Merged

Removed futuroscope #133

merged 1 commit into from
Jan 6, 2018

Conversation

dblock
Copy link
Collaborator

@dblock dblock commented Jan 5, 2018

Closes #123.

@ivoanjo Your call to merge!

@dblock dblock force-pushed the futuroscope branch 2 times, most recently from 71adb44 to bb6d631 Compare January 5, 2018 20:55
@dblock dblock requested a review from ivoanjo January 5, 2018 20:57
@dblock dblock force-pushed the futuroscope branch 6 times, most recently from 8e19b13 to f90a92a Compare January 5, 2018 21:55
@dblock dblock mentioned this pull request Jan 5, 2018
Copy link
Contributor

@ivoanjo ivoanjo left a comment

Choose a reason for hiding this comment

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

LGTM 👍

UPGRADING.md Outdated
end
```

The default new behavior is synchronous. Include `futuroscope` in your `Gemfile` and wrap Hyperclient requests into `Futuroscope::Future.new` blocks when you wish to utilize this pattern.
Copy link
Contributor

Choose a reason for hiding this comment

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

May I suggest we point people at concurrent-ruby for the reasons I discussed on #123?
It's equivalent in this case is wrapping the request with Concurrent::Future.execute.

@dblock
Copy link
Collaborator Author

dblock commented Jan 5, 2018

I wrote this. I think that's what we want.

The default new behavior is synchronous. We recommend concurrent-ruby for your future asynchronous needs.

Include futuroscope in your Gemfile and wrap Hyperclient requests into Futuroscope::Future.new blocks for the old behavior.

Just hit merge if you're happy with it ;)

@ivoanjo
Copy link
Contributor

ivoanjo commented Jan 6, 2018

Merging away!

@ivoanjo ivoanjo merged commit 5a522c8 into codegram:master Jan 6, 2018
@dblock dblock deleted the futuroscope branch January 10, 2018 21:54
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