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

Added an implicit parameter for the ExecutionContext #53

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

breckcs
Copy link

@breckcs breckcs commented Oct 13, 2016

  • Rather than importing the global ExecutionContext, allow the caller to
    specify the ExecutionContext.
  • Changed the minor version to 1.2.0 since this is a breaking change to the
    API. All the caller needs to do to maintain the existing behaviour, however,
    is import the global ExecutionContext, if it has not already been imported,
    as evidenced by the changes to the tests in this commit.

- Rather than importing the global ExecutionContext, allow the caller to
  specify the ExecutionContext.
- Changed the minor version to 1.2.0 since this is a breaking change to the
  API. All the caller needs to do to maintain the existing behaviour, however,
  is import the global ExecutionContext, if it has not already been imported,
  as evidenced by the changes to the tests in this commit.
@mkiedys
Copy link
Contributor

mkiedys commented Nov 18, 2016

I feel that we can retain compatibility while also allowing people to pass their own execution context. Akka faced similar problem, and they way they are doing it is quite good. Can you look at ActorSystem companion object?

We could still use default shared thread pool of two by default that gets disposed after last connection is closed but also allow people to pass their own ExecutionContext that they control and shutdown after it is no longer needed. It is both more convenient and most likely more performant solution to have some sane defaults.

@mkiedys mkiedys force-pushed the master branch 3 times, most recently from 9d5fb44 to 531d688 Compare November 18, 2016 12:12
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.

None yet

2 participants