Skip to content
This repository has been archived by the owner on Aug 6, 2020. It is now read-only.

Remove ExecutionContext prepare #22

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

pjfanning
Copy link

@pjfanning pjfanning commented Jan 18, 2019

Pull Request Checklist

  • Have you read through the contributor guidelines?
  • Have you signed the Typesafe CLA?
  • Have you squashed your commits?
  • Have you added copyright headers to new files?
  • Have you checked that both Scala and Java APIs are updated?
  • Have you updated the documentation for both Scala and Java sections?
  • Have you added tests for any changed functionality?

Fixes

Fixes #21

Purpose

What does this PR do?

Removes the use of ExecutionContext#prepare

Background Context

Why did you take this approach?

Trying to remove deprecated functions.

Pre-supposes #23 will be merged. If that PR is a problem, I can rework this PR.

References

Are there any relevant issues / PRs / mailing lists discussions?

#21

@masahitojp
Copy link
Contributor

hi @pjfanning
I am waiting for this change to be merged.
Could you please fix the conflict?

@pjfanning
Copy link
Author

@masahitojp is this change ok? Github now has a squash option for merges so I haven't tried to squash the commits myself.

@masahitojp
Copy link
Contributor

@pjfanning Thanks! I think it's ok!

@masahitojp
Copy link
Contributor

How about this? @marcospereira. I think we should follow Play 2.7 and make it usable with Scala 2.13.x

@dataich
Copy link

dataich commented Jun 10, 2019

+1

@@ -287,8 +287,6 @@ object Concurrent {
* $paramEcSingle
*/
def buffer[E](maxBuffer: Int, length: Input[E] => Int)(implicit ec: ExecutionContext): Enumeratee[E, E] = new Enumeratee[E, E] {
val pec = ec.prepare()
Copy link
Member

Choose a reason for hiding this comment

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

If required for other versions, it cannot be removed as-it or will break back compat

Copy link
Author

Choose a reason for hiding this comment

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

The use of the prepare function is not encouraged.

  /** Prepares for the execution of a task. Returns the prepared
     *  execution context. The recommended implementation of
     *  `prepare` is to return `this`.
     *
     *  This method should no longer be overridden or called. It was
     *  originally expected that `prepare` would be called by
     *  all libraries that consume ExecutionContexts, in order to
     *  capture thread local context. However, this usage has proven
     *  difficult to implement in practice and instead it is
     *  now better to avoid using `prepare` entirely.
     *
     *  Instead, if an `ExecutionContext` needs to capture thread
     *  local context, it should capture that context when it is
     *  constructed, so that it doesn't need any additional
     *  preparation later.
     */

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

remove ExecutionContext prepare calls
5 participants