You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@pekko.apache.org by "mdedetrich (via GitHub)" <gi...@apache.org> on 2023/04/06 15:06:32 UTC
[GitHub] [incubator-pekko] mdedetrich opened a new pull request, #281: Add FunctionConverters, FutureConverters and ObjectConverters
mdedetrich opened a new pull request, #281:
URL: https://github.com/apache/incubator-pekko/pull/281
The primary goal of this PR is two things
* Get rid of the `scala-java8-compat` library dependency if you are using newer versions of Scala where its not necessary (Scala 2.13 and newer specifically), see https://github.com/scala/scala-java8-compat#do-you-need-this .
* Make it easier to handle the dropping of Scala 2.12 support. This is due to the fact that since we are currently using `scala-java8-compat`, some of the syntax in `scala-java8-compat` doesn't exist in `scala.jdk` (which is the replacement for `scala-java8-compat` on Scala 2.13 and newer). In this PR, care has been taken when adding `FunctionConverters`, `FutureConverters` and `ObjectConverters` to only use methods/extension methods that exist or are not deprecated for Scala 2.13+
* This is the reason why, for example, `[asJavaPredicate(s => Objects.isNull(s))](predicate: Predicate[State] = asJavaPredicate(s => Objects.isNull(s)))` has been replaced with `predicate = ((s: State) => Objects.isNull(s)).asJava` since
`.asJava` is the only method that exists in both `scala.jdk` and `scala-java8-compat`.
* Also explains why method calls were changed from `asScala` to `toScala`, its because `asScala` is deprecated
* `OptionConverters.toScala` doesn't exist in Scala 2.13
In general this was achieved by delegating `scala-java8-compat` only for Scala 2.12 (and `scala-java8-compat` is only included in pekko for Scala 2.12) and delegating to `scala.jdk` for Scala 2.13+ (`scala.jdk` is included automatically since its in the scala runtime).
This feature will have an impact on end users, specifically in these cases
* They are using Scala 2.13+ and transitively relying on `scala-java8-compat` as a dependency.
* They are using Scala 2.13+ and are explicitly relying on an older binary incompatible `scala-java8-compat` as a dependency.
Both of these cases have been documented in the migration guide.
One thing to discuss is when this PR should be merged (i.e. for Pekko 1.0.x or 1.1.x). There are arguments either way, doing it earlier in 1.0.x means that its less painful for end users when we drop support for Scala 2.12 since Scala 2.13+ will not contain `scala-java8-compat`, so end users don't have to worry about it at all. On the other hand it is technically speaking a breaking change, however it can be argued that in the cases of breaking changes you should have migrated to `scala.jdk` and sbt (via eviction warnings)/compiler will warn you in such cases. Furthermore we have already done some breaking changes in Pekko anyways (when it comes to dependencies, ergo Jackson).
Also do note that when this PR will be merged we will have to update all of the Pekko modules since the Pekko modules are cross compiled against Scala 2.12/Scala 2.13 so even if this PR is approved, a convenient time needs to be chosen for when to merge this.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
[GitHub] [incubator-pekko] pjfanning commented on pull request #281: Add FunctionConverters, FutureConverters and ObjectConverters
Posted by "pjfanning (via GitHub)" <gi...@apache.org>.
pjfanning commented on PR #281:
URL: https://github.com/apache/incubator-pekko/pull/281#issuecomment-1538106729
sure - it could be a week or 2 before scala 3.3 comes out anyway
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
[GitHub] [incubator-pekko] He-Pin commented on a diff in pull request #281: Add FunctionConverters, FutureConverters and ObjectConverters
Posted by "He-Pin (via GitHub)" <gi...@apache.org>.
He-Pin commented on code in PR #281:
URL: https://github.com/apache/incubator-pekko/pull/281#discussion_r1159993770
##########
actor/src/main/scala/org/apache/pekko/actor/Actor.scala:
##########
@@ -100,8 +100,8 @@ final case class ActorIdentity(correlationId: Any, ref: Option[ActorRef]) {
* not defined if no actor matched the request.
*/
def getActorRef: Optional[ActorRef] = {
- import scala.compat.java8.OptionConverters._
- ref.asJava
+ import pekko.util.OptionConverters._
+ ref.toJava
Review Comment:
I see, thanks for the detail description.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
[GitHub] [incubator-pekko] He-Pin commented on a diff in pull request #281: Add FunctionConverters, FutureConverters and ObjectConverters
Posted by "He-Pin (via GitHub)" <gi...@apache.org>.
He-Pin commented on code in PR #281:
URL: https://github.com/apache/incubator-pekko/pull/281#discussion_r1160013407
##########
actor-testkit-typed/src/main/scala/org/apache/pekko/actor/testkit/typed/LoggingEvent.scala:
##########
@@ -15,13 +15,12 @@ package org.apache.pekko.actor.testkit.typed
import java.util.Optional
-import scala.compat.java8.OptionConverters._
-
import org.slf4j.Marker
import org.slf4j.event.Level
import org.apache.pekko
import pekko.util.ccompat.JavaConverters._
+import pekko.util.OptionConverters._
Review Comment:
Should it live in `pekko.util.ccompat.` too?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
[GitHub] [incubator-pekko] He-Pin commented on pull request #281: Add FunctionConverters, FutureConverters and ObjectConverters
Posted by "He-Pin (via GitHub)" <gi...@apache.org>.
He-Pin commented on PR #281:
URL: https://github.com/apache/incubator-pekko/pull/281#issuecomment-1519097265
Cool, then, i think we can provide more than a carbon copy of 2.6.20。
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
[GitHub] [incubator-pekko] He-Pin commented on a diff in pull request #281: Add FunctionConverters, FutureConverters and ObjectConverters
Posted by "He-Pin (via GitHub)" <gi...@apache.org>.
He-Pin commented on code in PR #281:
URL: https://github.com/apache/incubator-pekko/pull/281#discussion_r1159993770
##########
actor/src/main/scala/org/apache/pekko/actor/Actor.scala:
##########
@@ -100,8 +100,8 @@ final case class ActorIdentity(correlationId: Any, ref: Option[ActorRef]) {
* not defined if no actor matched the request.
*/
def getActorRef: Optional[ActorRef] = {
- import scala.compat.java8.OptionConverters._
- ref.asJava
+ import pekko.util.OptionConverters._
+ ref.toJava
Review Comment:
so all the `as` changed to `to`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
[GitHub] [incubator-pekko] He-Pin commented on a diff in pull request #281: Add FunctionConverters, FutureConverters and ObjectConverters
Posted by "He-Pin (via GitHub)" <gi...@apache.org>.
He-Pin commented on code in PR #281:
URL: https://github.com/apache/incubator-pekko/pull/281#discussion_r1160043760
##########
actor/src/main/scala/org/apache/pekko/actor/Actor.scala:
##########
@@ -100,8 +100,8 @@ final case class ActorIdentity(correlationId: Any, ref: Option[ActorRef]) {
* not defined if no actor matched the request.
*/
def getActorRef: Optional[ActorRef] = {
- import scala.compat.java8.OptionConverters._
- ref.asJava
+ import pekko.util.OptionConverters._
+ ref.toJava
Review Comment:
@mdedetrich I was trying to find a class named `ObjectConverters`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
[GitHub] [incubator-pekko] mdedetrich commented on a diff in pull request #281: Add FunctionConverters, FutureConverters and ObjectConverters
Posted by "mdedetrich (via GitHub)" <gi...@apache.org>.
mdedetrich commented on code in PR #281:
URL: https://github.com/apache/incubator-pekko/pull/281#discussion_r1160034804
##########
actor/src/main/scala/org/apache/pekko/actor/Actor.scala:
##########
@@ -100,8 +100,8 @@ final case class ActorIdentity(correlationId: Any, ref: Option[ActorRef]) {
* not defined if no actor matched the request.
*/
def getActorRef: Optional[ActorRef] = {
- import scala.compat.java8.OptionConverters._
- ref.asJava
+ import pekko.util.OptionConverters._
+ ref.toJava
Review Comment:
> And I think the title should be `OptionConverters `?
Don't understand what you mean by this? Is there a typo or are you thinking of something else.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
[GitHub] [incubator-pekko] mdedetrich commented on pull request #281: Add FunctionConverters, FutureConverters and ObjectConverters
Posted by "mdedetrich (via GitHub)" <gi...@apache.org>.
mdedetrich commented on PR #281:
URL: https://github.com/apache/incubator-pekko/pull/281#issuecomment-1519093782
> Is there any ETA of 1.1.0？
No idea, but it would be good to get an broad answer to my question before we start considering a 1.0.0 release because as both mentioned here and on the mailing list there are good reasons to put this change into 1.0.0.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
[GitHub] [incubator-pekko] mdedetrich merged pull request #281: Add FunctionConverters, FutureConverters and ObjectConverters
Posted by "mdedetrich (via GitHub)" <gi...@apache.org>.
mdedetrich merged PR #281:
URL: https://github.com/apache/incubator-pekko/pull/281
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
[GitHub] [incubator-pekko] mdedetrich commented on pull request #281: Add FunctionConverters, FutureConverters and ObjectConverters
Posted by "mdedetrich (via GitHub)" <gi...@apache.org>.
mdedetrich commented on PR #281:
URL: https://github.com/apache/incubator-pekko/pull/281#issuecomment-1538107581
Okay merging now then.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
[GitHub] [incubator-pekko] He-Pin commented on a diff in pull request #281: Add FunctionConverters, FutureConverters and ObjectConverters
Posted by "He-Pin (via GitHub)" <gi...@apache.org>.
He-Pin commented on code in PR #281:
URL: https://github.com/apache/incubator-pekko/pull/281#discussion_r1160038319
##########
persistence-typed/src/main/scala/org/apache/pekko/persistence/typed/javadsl/CommandHandlerWithReply.scala:
##########
@@ -117,7 +116,7 @@ final class CommandHandlerWithReplyBuilder[Command, Event, State]() {
* @return A new, mutable, CommandHandlerWithReplyBuilderByState
*/
def forNullState(): CommandHandlerWithReplyBuilderByState[Command, Event, State, State] = {
- val predicate: Predicate[State] = asJavaPredicate(s => Objects.isNull(s))
+ val predicate = ((s: State) => Objects.isNull(s)).asJava
Review Comment:
Btw, Why can't this be ：
```scala
val predicate: Predicate[State] = (s: State) => Objects.isNull(s)
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
[GitHub] [incubator-pekko] He-Pin commented on a diff in pull request #281: Add FunctionConverters, FutureConverters and ObjectConverters
Posted by "He-Pin (via GitHub)" <gi...@apache.org>.
He-Pin commented on code in PR #281:
URL: https://github.com/apache/incubator-pekko/pull/281#discussion_r1160038319
##########
persistence-typed/src/main/scala/org/apache/pekko/persistence/typed/javadsl/CommandHandlerWithReply.scala:
##########
@@ -117,7 +116,7 @@ final class CommandHandlerWithReplyBuilder[Command, Event, State]() {
* @return A new, mutable, CommandHandlerWithReplyBuilderByState
*/
def forNullState(): CommandHandlerWithReplyBuilderByState[Command, Event, State, State] = {
- val predicate: Predicate[State] = asJavaPredicate(s => Objects.isNull(s))
+ val predicate = ((s: State) => Objects.isNull(s)).asJava
Review Comment:
Btw, Why can this be ：
```scala
val predicate: Predicate[State] = (s: State) => Objects.isNull(s)
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
[GitHub] [incubator-pekko] He-Pin commented on a diff in pull request #281: Add FunctionConverters, FutureConverters and ObjectConverters
Posted by "He-Pin (via GitHub)" <gi...@apache.org>.
He-Pin commented on code in PR #281:
URL: https://github.com/apache/incubator-pekko/pull/281#discussion_r1160066031
##########
project/Dependencies.scala:
##########
@@ -237,7 +229,13 @@ object Dependencies {
// TODO check if `l ++=` everywhere expensive?
val l = libraryDependencies
- val actor = l ++= Seq(config, java8Compat.value)
+ val actor = l ++= (CrossVersion.partialVersion(scalaVersion.value) match {
+ // java8-compat is only used in a couple of places for 2.13,
+ // it is probably possible to remove the dependency if needed.
+ case Some((2, n)) if n == 12 =>
+ List("org.scala-lang.modules" %% "scala-java8-compat" % java8CompatVersion.value) // Scala License
+ case _ => List.empty
+ }) ++ Seq(config)
Review Comment:
Cool thing.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
[GitHub] [incubator-pekko] mdedetrich commented on a diff in pull request #281: Add FunctionConverters, FutureConverters and ObjectConverters
Posted by "mdedetrich (via GitHub)" <gi...@apache.org>.
mdedetrich commented on code in PR #281:
URL: https://github.com/apache/incubator-pekko/pull/281#discussion_r1160051495
##########
actor/src/main/scala/org/apache/pekko/actor/Actor.scala:
##########
@@ -100,8 +100,8 @@ final case class ActorIdentity(correlationId: Any, ref: Option[ActorRef]) {
* not defined if no actor matched the request.
*/
def getActorRef: Optional[ActorRef] = {
- import scala.compat.java8.OptionConverters._
- ref.asJava
+ import pekko.util.OptionConverters._
+ ref.toJava
Review Comment:
Ah so you can find it when reviewing?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
[GitHub] [incubator-pekko] pjfanning commented on pull request #281: Add FunctionConverters, FutureConverters and ObjectConverters
Posted by "pjfanning (via GitHub)" <gi...@apache.org>.
pjfanning commented on PR #281:
URL: https://github.com/apache/incubator-pekko/pull/281#issuecomment-1519094298
We are a long way from v1.1. Whenever v1.0.0 is released, we can split branches - 1.0 support and 1.1 support - with their own snapshots.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
[GitHub] [incubator-pekko] mdedetrich commented on pull request #281: Add FunctionConverters, FutureConverters and ObjectConverters
Posted by "mdedetrich (via GitHub)" <gi...@apache.org>.
mdedetrich commented on PR #281:
URL: https://github.com/apache/incubator-pekko/pull/281#issuecomment-1538104050
@pjfanning I want to go ahead and merge this today. Is it okay if we delay the MILESTONE release until we find out that this PR works with every Pekko module (I will go ahead and update the modules myself). There may be a chance that I might miss one of the collection compat methods and if that happens then I will have to go back and add some more changes into Pekko.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
[GitHub] [incubator-pekko] He-Pin commented on a diff in pull request #281: Add FunctionConverters, FutureConverters and ObjectConverters
Posted by "He-Pin (via GitHub)" <gi...@apache.org>.
He-Pin commented on code in PR #281:
URL: https://github.com/apache/incubator-pekko/pull/281#discussion_r1159997875
##########
actor/src/main/scala/org/apache/pekko/actor/Actor.scala:
##########
@@ -100,8 +100,8 @@ final case class ActorIdentity(correlationId: Any, ref: Option[ActorRef]) {
* not defined if no actor matched the request.
*/
def getActorRef: Optional[ActorRef] = {
- import scala.compat.java8.OptionConverters._
- ref.asJava
+ import pekko.util.OptionConverters._
+ ref.toJava
Review Comment:
~~And I think the title should be `OptionConverters `?~~
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
[GitHub] [incubator-pekko] mdedetrich commented on a diff in pull request #281: Add FunctionConverters, FutureConverters and ObjectConverters
Posted by "mdedetrich (via GitHub)" <gi...@apache.org>.
mdedetrich commented on code in PR #281:
URL: https://github.com/apache/incubator-pekko/pull/281#discussion_r1160138688
##########
actor/src/main/scala/org/apache/pekko/actor/Actor.scala:
##########
@@ -100,8 +100,8 @@ final case class ActorIdentity(correlationId: Any, ref: Option[ActorRef]) {
* not defined if no actor matched the request.
*/
def getActorRef: Optional[ActorRef] = {
- import scala.compat.java8.OptionConverters._
- ref.asJava
+ import pekko.util.OptionConverters._
+ ref.toJava
Review Comment:
My bad, a had a mental blip. `ObjectConverters` is meant to be `OptionConverters`. Edited original post to also reflect this.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
[GitHub] [incubator-pekko] He-Pin commented on pull request #281: Add FunctionConverters, FutureConverters and ObjectConverters
Posted by "He-Pin (via GitHub)" <gi...@apache.org>.
He-Pin commented on PR #281:
URL: https://github.com/apache/incubator-pekko/pull/281#issuecomment-1499278022
i think the overall design is good but we said pekko 1.0 should be a carbon copy of 2.6.20, so defer this to 1.1.0？
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
[GitHub] [incubator-pekko] mdedetrich commented on a diff in pull request #281: Add FunctionConverters, FutureConverters and ObjectConverters
Posted by "mdedetrich (via GitHub)" <gi...@apache.org>.
mdedetrich commented on code in PR #281:
URL: https://github.com/apache/incubator-pekko/pull/281#discussion_r1160034187
##########
actor-testkit-typed/src/main/scala/org/apache/pekko/actor/testkit/typed/LoggingEvent.scala:
##########
@@ -15,13 +15,12 @@ package org.apache.pekko.actor.testkit.typed
import java.util.Optional
-import scala.compat.java8.OptionConverters._
-
import org.slf4j.Marker
import org.slf4j.event.Level
import org.apache.pekko
import pekko.util.ccompat.JavaConverters._
+import pekko.util.OptionConverters._
Review Comment:
So the reason why I moved these converter libraries outside of `ccompat` is that since `ccompat` is a package object it can't be accessed within Java (this was the case for `FutureConverters`).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
[GitHub] [incubator-pekko] mdedetrich commented on pull request #281: Add FunctionConverters, FutureConverters and ObjectConverters
Posted by "mdedetrich (via GitHub)" <gi...@apache.org>.
mdedetrich commented on PR #281:
URL: https://github.com/apache/incubator-pekko/pull/281#issuecomment-1519018315
@jrudolph @raboof Would it be possible to get your opinion on this? The decision can affect if we want to land this before 1.0.0 or after
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
[GitHub] [incubator-pekko] He-Pin commented on pull request #281: Add FunctionConverters, FutureConverters and ObjectConverters
Posted by "He-Pin (via GitHub)" <gi...@apache.org>.
He-Pin commented on PR #281:
URL: https://github.com/apache/incubator-pekko/pull/281#issuecomment-1519091706
Is there any ETA of 1.1.0？
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
[GitHub] [incubator-pekko] pjfanning commented on pull request #281: Add FunctionConverters, FutureConverters and ObjectConverters
Posted by "pjfanning (via GitHub)" <gi...@apache.org>.
pjfanning commented on PR #281:
URL: https://github.com/apache/incubator-pekko/pull/281#issuecomment-1499267815
Could we consider delaying this to v1.1?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
[GitHub] [incubator-pekko] pjfanning commented on pull request #281: Add FunctionConverters, FutureConverters and ObjectConverters
Posted by "pjfanning (via GitHub)" <gi...@apache.org>.
pjfanning commented on PR #281:
URL: https://github.com/apache/incubator-pekko/pull/281#issuecomment-1499317386
I guess that I would be happy enough to merge this now. The conversions are internal - to support the Pekko Java API. And the new conversion functions are marked as internal.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
[GitHub] [incubator-pekko] He-Pin commented on a diff in pull request #281: Add FunctionConverters, FutureConverters and ObjectConverters
Posted by "He-Pin (via GitHub)" <gi...@apache.org>.
He-Pin commented on code in PR #281:
URL: https://github.com/apache/incubator-pekko/pull/281#discussion_r1160057805
##########
persistence-typed/src/main/scala/org/apache/pekko/persistence/typed/javadsl/CommandHandlerWithReply.scala:
##########
@@ -117,7 +116,7 @@ final class CommandHandlerWithReplyBuilder[Command, Event, State]() {
* @return A new, mutable, CommandHandlerWithReplyBuilderByState
*/
def forNullState(): CommandHandlerWithReplyBuilderByState[Command, Event, State, State] = {
- val predicate: Predicate[State] = asJavaPredicate(s => Objects.isNull(s))
+ val predicate = ((s: State) => Objects.isNull(s)).asJava
Review Comment:
Thanks, so the `predicate` is been called in `scaladsl`? I'm not familiar with persistence.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
[GitHub] [incubator-pekko] He-Pin commented on a diff in pull request #281: Add FunctionConverters, FutureConverters and ObjectConverters
Posted by "He-Pin (via GitHub)" <gi...@apache.org>.
He-Pin commented on code in PR #281:
URL: https://github.com/apache/incubator-pekko/pull/281#discussion_r1159997875
##########
actor/src/main/scala/org/apache/pekko/actor/Actor.scala:
##########
@@ -100,8 +100,8 @@ final case class ActorIdentity(correlationId: Any, ref: Option[ActorRef]) {
* not defined if no actor matched the request.
*/
def getActorRef: Optional[ActorRef] = {
- import scala.compat.java8.OptionConverters._
- ref.asJava
+ import pekko.util.OptionConverters._
+ ref.toJava
Review Comment:
And I think the title should be `OptionConverters `?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
[GitHub] [incubator-pekko] mdedetrich commented on a diff in pull request #281: Add FunctionConverters, FutureConverters and ObjectConverters
Posted by "mdedetrich (via GitHub)" <gi...@apache.org>.
mdedetrich commented on code in PR #281:
URL: https://github.com/apache/incubator-pekko/pull/281#discussion_r1160034187
##########
actor-testkit-typed/src/main/scala/org/apache/pekko/actor/testkit/typed/LoggingEvent.scala:
##########
@@ -15,13 +15,12 @@ package org.apache.pekko.actor.testkit.typed
import java.util.Optional
-import scala.compat.java8.OptionConverters._
-
import org.slf4j.Marker
import org.slf4j.event.Level
import org.apache.pekko
import pekko.util.ccompat.JavaConverters._
+import pekko.util.OptionConverters._
Review Comment:
So the reason why I moved these converter libraries outside of `ccompat` is that since `ccompat` is a package object it can't be accessed within Java (this was the case for `FutureConverters` which is used within a Java test).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
[GitHub] [incubator-pekko] mdedetrich commented on a diff in pull request #281: Add FunctionConverters, FutureConverters and ObjectConverters
Posted by "mdedetrich (via GitHub)" <gi...@apache.org>.
mdedetrich commented on code in PR #281:
URL: https://github.com/apache/incubator-pekko/pull/281#discussion_r1160085550
##########
persistence-typed/src/main/scala/org/apache/pekko/persistence/typed/javadsl/CommandHandlerWithReply.scala:
##########
@@ -117,7 +116,7 @@ final class CommandHandlerWithReplyBuilder[Command, Event, State]() {
* @return A new, mutable, CommandHandlerWithReplyBuilderByState
*/
def forNullState(): CommandHandlerWithReplyBuilderByState[Command, Event, State, State] = {
- val predicate: Predicate[State] = asJavaPredicate(s => Objects.isNull(s))
+ val predicate = ((s: State) => Objects.isNull(s)).asJava
Review Comment:
Yes, the type needs to be `Predicate[State]` (which is actually a Java `Functional` interface)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
[GitHub] [incubator-pekko] mdedetrich commented on pull request #281: Add FunctionConverters, FutureConverters and ObjectConverters
Posted by "mdedetrich (via GitHub)" <gi...@apache.org>.
mdedetrich commented on PR #281:
URL: https://github.com/apache/incubator-pekko/pull/281#issuecomment-1499271020
> Could we consider delaying this to v1.1?
I wrote a paragraph on this, there are pros and cons to delaying this to 1.1. Personally when I ran through the cases in my head about how this could theoretically break, I couldn't come to a scenario where a user wouldn't be notified. The cases that would occur is that compile would no longer work (where as documented you would just add `scala-java8-compat` to your dependencies)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
[GitHub] [incubator-pekko] mdedetrich commented on pull request #281: Add FunctionConverters, FutureConverters and ObjectConverters
Posted by "mdedetrich (via GitHub)" <gi...@apache.org>.
mdedetrich commented on PR #281:
URL: https://github.com/apache/incubator-pekko/pull/281#issuecomment-1499279000
> pekko 1.0 should be a carbon copy of 2.6.20, so defer this to 1.1.0？
This really isn't that true anymore, also I edited my previous point but one good argument for 1.0.x is if we decide to drop Scala 2.12 support in 1.1.x.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
[GitHub] [incubator-pekko] He-Pin commented on a diff in pull request #281: Add FunctionConverters, FutureConverters and ObjectConverters
Posted by "He-Pin (via GitHub)" <gi...@apache.org>.
He-Pin commented on code in PR #281:
URL: https://github.com/apache/incubator-pekko/pull/281#discussion_r1160018053
##########
persistence-typed/src/main/scala/org/apache/pekko/persistence/typed/javadsl/CommandHandlerWithReply.scala:
##########
@@ -117,7 +116,7 @@ final class CommandHandlerWithReplyBuilder[Command, Event, State]() {
* @return A new, mutable, CommandHandlerWithReplyBuilderByState
*/
def forNullState(): CommandHandlerWithReplyBuilderByState[Command, Event, State, State] = {
- val predicate: Predicate[State] = asJavaPredicate(s => Objects.isNull(s))
+ val predicate = ((s: State) => Objects.isNull(s)).asJava
Review Comment:
Hah, I see
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
[GitHub] [incubator-pekko] mdedetrich commented on a diff in pull request #281: Add FunctionConverters, FutureConverters and ObjectConverters
Posted by "mdedetrich (via GitHub)" <gi...@apache.org>.
mdedetrich commented on code in PR #281:
URL: https://github.com/apache/incubator-pekko/pull/281#discussion_r1160040187
##########
persistence-typed/src/main/scala/org/apache/pekko/persistence/typed/javadsl/CommandHandlerWithReply.scala:
##########
@@ -117,7 +116,7 @@ final class CommandHandlerWithReplyBuilder[Command, Event, State]() {
* @return A new, mutable, CommandHandlerWithReplyBuilderByState
*/
def forNullState(): CommandHandlerWithReplyBuilderByState[Command, Event, State, State] = {
- val predicate: Predicate[State] = asJavaPredicate(s => Objects.isNull(s))
+ val predicate = ((s: State) => Objects.isNull(s)).asJava
Review Comment:
> Btw, Why can this be ：
>
> ```scala
> val predicate: Predicate[State] = (s: State) => Objects.isNull(s)
> ```
Nope, there aren't implicit conversions. `(s: State) => Objects.isNull(s)` is of type `Function1[State, Boolean]` and the .`asJava` turns that into a `Predicate[State]`).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
[GitHub] [incubator-pekko] mdedetrich commented on pull request #281: Add FunctionConverters, FutureConverters and ObjectConverters
Posted by "mdedetrich (via GitHub)" <gi...@apache.org>.
mdedetrich commented on PR #281:
URL: https://github.com/apache/incubator-pekko/pull/281#issuecomment-1519095232
> I support adding this PR now since it has limited impact on the API.
In that case if you are happy to do so can you review the PR? I wont merge it immediately because I do genuinely want feedback from others but if no one comments on it by the time we actually do a release I will take it as an implicit acceptance.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
[GitHub] [incubator-pekko] pjfanning commented on pull request #281: Add FunctionConverters, FutureConverters and ObjectConverters
Posted by "pjfanning (via GitHub)" <gi...@apache.org>.
pjfanning commented on PR #281:
URL: https://github.com/apache/incubator-pekko/pull/281#issuecomment-1519094509
I support adding this PR now since it has limited impact on the API.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org