Skip to content

More efficient round trip conversions for Futures #50

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

Merged
merged 1 commit into from
Aug 28, 2015

Conversation

retronym
Copy link
Member

A straw man proposal, for discussion by @jroper @rkuhn and
@viktorklang.

See the second commit for details, as I've based this PR on
another in-flight PR.

Fixes #44

@@ -6,7 +6,7 @@ package scala.concurrent.java8
// Located in this package to access private[concurrent] members

import scala.concurrent.{ Future, Promise, ExecutionContext, ExecutionContextExecutorService, ExecutionContextExecutor, impl }
import java.util.concurrent.{ CompletionStage, Executor, ExecutorService, CompletableFuture }
import java.util.concurrent._

Choose a reason for hiding this comment

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

Aren't you shadowing scala.concurrent.Future now?

Copy link
Member Author

Choose a reason for hiding this comment

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

Nope, wildcards have a lower precendence:

scala> {import scala.concurrent.Future; import java.util.concurrent._; 0 : Future[_]}
<console>:8: error: type mismatch;
 found   : Int(0)
 required: scala.concurrent.Future[_]
              {import scala.concurrent.Future; import java.util.concurrent._; 0 : Future[_]}
                                                                              ^

retronym added a commit to retronym/scala that referenced this pull request Aug 13, 2015
Doing so seems like the cleanest way to enabled conversions like
`javaFuture.toScala.toJava` to return the original `javaFuture`
in scala-java8-compat.

I have made the methods defined in this class final as an
alternative lockdown.

Discussion, Motivation:

  scala/scala-java8-compat#46
  scala/scala-java8-compat#50
retronym added a commit to retronym/scala that referenced this pull request Aug 13, 2015
It was non-final in Scala 2.11.x, and made final as part
of fa0743c.

Removing the final modifier seems like the cleanest way to enable
conversions like `javaFuture.toScala.toJava` to return the original
`javaFuture` in scala-java8-compat.

I have made the methods defined in this class final as an
alternative lockdown.

Discussion, Motivation:

  scala/scala-java8-compat#46
  scala/scala-java8-compat#50
@retronym retronym force-pushed the ticket/44 branch 3 times, most recently from 1967830 to a176d57 Compare August 18, 2015 05:32
Makes `toJava.toScala` and `toScala.toJava` a wrap-and-unwrap
operation, rather than a wrap-and-rewrap.

Removes the cross build on 2.12, we should wait until 2.12.0-M3
in which DefaultPromise is once again extendable.

See:
  4faeac5
  scala/scala#4690
@jroper
Copy link

jroper commented Aug 28, 2015

This looks good to me.

retronym added a commit that referenced this pull request Aug 28, 2015
More efficient round trip conversions for Futures
@retronym retronym merged commit c1ae1c1 into scala:master Aug 28, 2015
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.

3 participants