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
Use scala.concurrent.Batchable for Scala 2.13 #29986
Use scala.concurrent.Batchable for Scala 2.13 #29986
Conversation
Test FAILed. |
1 similar comment
Test FAILed. |
Test PASSed. |
PLS BUILD |
Test PASSed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
|
||
def isBatchable(runnable: Runnable): Boolean = runnable match { | ||
case b: akka.dispatch.Batchable => b.isBatchable | ||
case _: scala.concurrent.Batchable => true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm sure I fully understand what happened to the Scala internals as described in the comment above. What if this version of Akka is run in an application with Scala 2.13.3 dependency? Will it be caught by this scala.concurrent.Batchable
case, or will this be a regression compared to previous OnCompleteRunnable?
We might not care much about that and we can say that they should update to Scala 2.13.4. We already have another reason for that: #30018
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OnCompleteRunnable
extends Batchable
in all 2.13.x as far as I can see, so it should not cause a regression in older 2.13 releases.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, thanks
Is there any plan for backport to akka 2.5.x? 👀 |
I don't think we're planning a backport, though if someone would contribute one I don't see a reason not to accept it. Be aware we're not really planning a new 2.5.x release either, though (but perhaps a snapshot would be sufficient if you need it sooner). |
References #29980
Manually tested with 2.12.11 and 2.13.3 however 2.13.4 requires quite a lot of changes (deprecated methods in
IterableOps
, exhaustiveness check errors all over the place) and I didn't want to grow this PR so I let that be.With 2.13.4
impl.Promise.Transformation
no longer extends the deprecatedOnCompleteRunnable
directly but insteadscala.concurrent.Batchable
. HoweverOnCompleteRunnable
extendsBatchable
since 2.13.0 so matching againstBatchable
should be fine for all 2.13.x versions.