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/12/31 00:28:45 UTC
[PR] Remove Scala 3 inline keyword from Java specific conversion methods [incubator-pekko]
mdedetrich opened a new pull request, #894:
URL: https://github.com/apache/incubator-pekko/pull/894
The response in https://github.com/lampepfl/dotty/issues/19346#issuecomment-1872478851 confirms that its by design that methods marked `inline` in Scala 3 are not meant to be called within Java, it is intended that if you mark a method `inline` in Scala 3 that its meant to enforce/guarantee that this method is inlined which for obvious reasons is not possible if the method is called within Java. This is different from the `@inline` annotation in Scala 2 which you can freely call within Java, it just won't be inlined.
Due to this, this PR removes the Scala 3 `inline` keyword from methods which are designed to be called within Java as well as adding a test class to ensure that these designated Java methods can actually be compiled within a Java source. You can add the `inline` keyword to the relevant methods to confirm that this test properly catches this.
Going forward what this means is that if any of the code within Pekko projects happen to call these conversion methods, if the method is being called within a `*.scala` source then it should use the extension method versions (i.e. the methods defined within `implicit class`) in order to get the inline benefits.
--
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
Re: [PR] Remove Scala 3 inline keyword from Java specific conversion methods [incubator-pekko]
Posted by "pjfanning (via GitHub)" <gi...@apache.org>.
pjfanning commented on code in PR #894:
URL: https://github.com/apache/incubator-pekko/pull/894#discussion_r1438887702
##########
actor-tests/src/test/java/org/apache/pekko/util/JavaConverterScala3InlineTest.java:
##########
@@ -0,0 +1,39 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * license agreements; and to You under the Apache License, version 2.0:
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * This file is part of the Apache Pekko project, which was derived from Akka.
Review Comment:
brand new files should use the standard ASF header - unless it has code copied in from an existing file in which case it should use the headers from that file
--
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
Re: [PR] Remove Scala 3 inline keyword from Java specific conversion methods [incubator-pekko]
Posted by "mdedetrich (via GitHub)" <gi...@apache.org>.
mdedetrich commented on code in PR #894:
URL: https://github.com/apache/incubator-pekko/pull/894#discussion_r1438781036
##########
actor-tests/src/test/java/org/apache/pekko/util/JavaConverterScala3InlineTest.java:
##########
@@ -0,0 +1,39 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * license agreements; and to You under the Apache License, version 2.0:
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * This file is part of the Apache Pekko project, which was derived from Akka.
Review Comment:
@pjfanning Whats your views on this, original Apache header also makes sense but this header is the one copied from https://github.com/apache/incubator-pekko/blob/b0fdac259bd57fdd481483f3fe9a7aec6e1ff38a/actor-tests/src/test/scala/org/apache/pekko/util/Scala212CompatTest.scala#L1-L8 which is a comparable new test that was written?
--
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
Re: [PR] Remove Scala 3 inline keyword from Java specific conversion methods [incubator-pekko]
Posted by "laglangyue (via GitHub)" <gi...@apache.org>.
laglangyue commented on code in PR #894:
URL: https://github.com/apache/incubator-pekko/pull/894#discussion_r1438771717
##########
actor/src/main/scala/org/apache/pekko/actor/ActorSelection.scala:
##########
@@ -108,8 +108,10 @@ abstract class ActorSelection extends Serializable {
* supplied `timeout`.
*/
@deprecated("Use the overloaded method resolveOne which accepts java.time.Duration instead.", since = "Akka 2.5.20")
- def resolveOneCS(timeout: FiniteDuration): CompletionStage[ActorRef] =
- FutureConverters.asJava[ActorRef](resolveOne(timeout))
+ def resolveOneCS(timeout: FiniteDuration): CompletionStage[ActorRef] = {
+ import FutureConverters._
+ resolveOne(timeout).asJava
Review Comment:
this changs seems not related with this pr. The API here should be shown to Java programmers, and I think the original way is easier for Java programmers to understand.
--
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
Re: [PR] Remove Scala 3 inline keyword from Java specific conversion methods [incubator-pekko]
Posted by "mdedetrich (via GitHub)" <gi...@apache.org>.
mdedetrich commented on PR #894:
URL: https://github.com/apache/incubator-pekko/pull/894#issuecomment-1874720964
@pjfanning I can confirm that this PR did indeed fix the issue with pekko-connectors (see https://github.com/apache/incubator-pekko-connectors/actions/runs/7391882740)
--
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
Re: [PR] Remove Scala 3 inline keyword from Java specific conversion methods [incubator-pekko]
Posted by "laglangyue (via GitHub)" <gi...@apache.org>.
laglangyue commented on code in PR #894:
URL: https://github.com/apache/incubator-pekko/pull/894#discussion_r1438780228
##########
actor-tests/src/test/java/org/apache/pekko/util/JavaConverterScala3InlineTest.java:
##########
@@ -0,0 +1,39 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * license agreements; and to You under the Apache License, version 2.0:
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * This file is part of the Apache Pekko project, which was derived from Akka.
Review Comment:
this is a new File,can we remove the description about akka
--
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
Re: [PR] Remove Scala 3 inline keyword from Java specific conversion methods [incubator-pekko]
Posted by "mdedetrich (via GitHub)" <gi...@apache.org>.
mdedetrich commented on code in PR #894:
URL: https://github.com/apache/incubator-pekko/pull/894#discussion_r1438933114
##########
actor-tests/src/test/java/org/apache/pekko/util/JavaConverterScala3InlineTest.java:
##########
@@ -0,0 +1,39 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * license agreements; and to You under the Apache License, version 2.0:
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * This file is part of the Apache Pekko project, which was derived from Akka.
Review Comment:
> If anything Scala212CompatTest seems to have the wrong header - that file looks like it was written as a fresh set of tests by the Pekko team.
Thanks, ill fix the header for that file in another PR as well as fixing the headers in this PR
--
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
Re: [PR] Remove Scala 3 inline keyword from Java specific conversion methods [incubator-pekko]
Posted by "mdedetrich (via GitHub)" <gi...@apache.org>.
mdedetrich merged PR #894:
URL: https://github.com/apache/incubator-pekko/pull/894
--
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
Re: [PR] Remove Scala 3 inline keyword from Java specific conversion methods [incubator-pekko]
Posted by "mdedetrich (via GitHub)" <gi...@apache.org>.
mdedetrich commented on code in PR #894:
URL: https://github.com/apache/incubator-pekko/pull/894#discussion_r1438780792
##########
actor/src/main/scala/org/apache/pekko/actor/ActorSelection.scala:
##########
@@ -108,8 +108,10 @@ abstract class ActorSelection extends Serializable {
* supplied `timeout`.
*/
@deprecated("Use the overloaded method resolveOne which accepts java.time.Duration instead.", since = "Akka 2.5.20")
- def resolveOneCS(timeout: FiniteDuration): CompletionStage[ActorRef] =
- FutureConverters.asJava[ActorRef](resolveOne(timeout))
+ def resolveOneCS(timeout: FiniteDuration): CompletionStage[ActorRef] = {
+ import FutureConverters._
+ resolveOne(timeout).asJava
Review Comment:
> this changs seems not related with this pr.
It is, specifically this part from the PR description
> Going forward what this means is that if any of the code within Pekko projects happen to call these conversion methods, if the method is being called within a *.scala source then it should use the extension method versions (i.e. the methods defined within implicit final class) in order to get the inline benefits when compiled with Scala 3. Thats the best that can be done in this situation and it also explains the rest of the changes in this PR.
Also
> The API here should be shown to Java programmers, and I think the original way is easier for Java programmers to understand.
This change has zero effect on Java code calling `def resolveOneCS(timeout: FiniteDuration): CompletionStage[ActorRef]` or the APi, the inlining is only occurring in the internal implementation (if thats what you are were implying in your question).
--
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
Re: [PR] Remove Scala 3 inline keyword from Java specific conversion methods [incubator-pekko]
Posted by "mdedetrich (via GitHub)" <gi...@apache.org>.
mdedetrich commented on code in PR #894:
URL: https://github.com/apache/incubator-pekko/pull/894#discussion_r1438808477
##########
actor-tests/src/test/java/org/apache/pekko/util/JavaConverterScala3InlineTest.java:
##########
@@ -0,0 +1,39 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * license agreements; and to You under the Apache License, version 2.0:
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * This file is part of the Apache Pekko project, which was derived from Akka.
+ */
+
+package org.apache.pekko.util;
+
+import java.time.Duration;
+import java.util.*;
+import java.util.concurrent.CompletableFuture;
+
+import scala.Option;
+import scala.concurrent.Future;
+
+/**
+ * These tests are here to ensure that methods from {@link org.apache.pekko.util.FutureConverters},
+ * {@link org.apache.pekko.util.JavaDurationConverters} and {@link
+ * org.apache.pekko.util.OptionConverters} for use within Java can be compiled from with Java
+ * sources. This is because methods marked with the Scala 3 inline keyword cannot be called from
+ * within Java (see https://github.com/lampepfl/dotty/issues/19346)
+ */
+public class JavaConverterScala3InlineTest {
+ public void Test() {
+ OptionConverters.toScala(Optional.empty());
+ OptionConverters.toScala(OptionalDouble.of(0));
+ OptionConverters.toScala(OptionalInt.of(0));
+ OptionConverters.toScala(OptionalLong.of(0));
+ OptionConverters.toJava(Option.empty());
+
+ FutureConverters.asJava(Future.successful(""));
+ FutureConverters.asScala(CompletableFuture.completedFuture(""));
+
+ JavaDurationConverters.asFiniteDuration(Duration.ofMillis(0));
Review Comment:
Same as https://github.com/apache/incubator-pekko/pull/894#discussion_r1438808197
--
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
Re: [PR] Remove Scala 3 inline keyword from Java specific conversion methods [incubator-pekko]
Posted by "pjfanning (via GitHub)" <gi...@apache.org>.
pjfanning commented on code in PR #894:
URL: https://github.com/apache/incubator-pekko/pull/894#discussion_r1438891218
##########
actor-tests/src/test/java/org/apache/pekko/util/JavaConverterScala3InlineTest.java:
##########
@@ -0,0 +1,39 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * license agreements; and to You under the Apache License, version 2.0:
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * This file is part of the Apache Pekko project, which was derived from Akka.
Review Comment:
If anything Scala212CompatTest seems to have the wrong header - that file looks like it was written as a fresh set of tests by the Pekko team.
--
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
Re: [PR] Remove Scala 3 inline keyword from Java specific conversion methods [incubator-pekko]
Posted by "mdedetrich (via GitHub)" <gi...@apache.org>.
mdedetrich commented on code in PR #894:
URL: https://github.com/apache/incubator-pekko/pull/894#discussion_r1438781036
##########
actor-tests/src/test/java/org/apache/pekko/util/JavaConverterScala3InlineTest.java:
##########
@@ -0,0 +1,39 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * license agreements; and to You under the Apache License, version 2.0:
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * This file is part of the Apache Pekko project, which was derived from Akka.
Review Comment:
@pjfanning Whats your views on this, original Apache header also makes sense but this header is the one copied from https://github.com/apache/incubator-pekko/blob/b0fdac259bd57fdd481483f3fe9a7aec6e1ff38a/actor-tests/src/test/scala/org/apache/pekko/util/Scala212CompatTest.scala#L1-L8 which is a comparable new test that was written
--
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
Re: [PR] Remove Scala 3 inline keyword from Java specific conversion methods [incubator-pekko]
Posted by "pjfanning (via GitHub)" <gi...@apache.org>.
pjfanning commented on code in PR #894:
URL: https://github.com/apache/incubator-pekko/pull/894#discussion_r1438887882
##########
actor-tests/src/test/java/org/apache/pekko/util/JavaConverterScala3InlineTest.java:
##########
@@ -0,0 +1,39 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * license agreements; and to You under the Apache License, version 2.0:
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * This file is part of the Apache Pekko project, which was derived from Akka.
+ */
+
+package org.apache.pekko.util;
+
+import java.time.Duration;
+import java.util.*;
+import java.util.concurrent.CompletableFuture;
+
+import scala.Option;
+import scala.concurrent.Future;
+
+/**
+ * These tests are here to ensure that methods from {@link org.apache.pekko.util.FutureConverters},
+ * {@link org.apache.pekko.util.JavaDurationConverters} and {@link
+ * org.apache.pekko.util.OptionConverters} for use within Java can be compiled from with Java
+ * sources. This is because methods marked with the Scala 3 inline keyword cannot be called from
+ * within Java (see https://github.com/lampepfl/dotty/issues/19346)
+ */
+public class JavaConverterScala3InlineTest {
+ public void Test() {
Review Comment:
shouldn't method names start with lower case chars? - so `Test()` doesn't seem like a good method name
--
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
Re: [PR] Remove Scala 3 inline keyword from Java specific conversion methods [incubator-pekko]
Posted by "pjfanning (via GitHub)" <gi...@apache.org>.
pjfanning commented on code in PR #894:
URL: https://github.com/apache/incubator-pekko/pull/894#discussion_r1438887702
##########
actor-tests/src/test/java/org/apache/pekko/util/JavaConverterScala3InlineTest.java:
##########
@@ -0,0 +1,39 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * license agreements; and to You under the Apache License, version 2.0:
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * This file is part of the Apache Pekko project, which was derived from Akka.
Review Comment:
brand new files should use the standard ASF header - unless it has code copied in from an existing file in which case it should use the headers from that file
if you just use another file for guidance on imports and code structure then I think that is not enough for us to say we need to use the non-standard header (mentioning Akka).
--
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
Re: [PR] Remove Scala 3 inline keyword from Java specific conversion methods [incubator-pekko]
Posted by "He-Pin (via GitHub)" <gi...@apache.org>.
He-Pin commented on code in PR #894:
URL: https://github.com/apache/incubator-pekko/pull/894#discussion_r1438806244
##########
actor-tests/src/test/java/org/apache/pekko/util/JavaConverterScala3InlineTest.java:
##########
@@ -0,0 +1,39 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * license agreements; and to You under the Apache License, version 2.0:
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * This file is part of the Apache Pekko project, which was derived from Akka.
+ */
+
+package org.apache.pekko.util;
+
+import java.time.Duration;
+import java.util.*;
+import java.util.concurrent.CompletableFuture;
+
+import scala.Option;
+import scala.concurrent.Future;
+
+/**
+ * These tests are here to ensure that methods from {@link org.apache.pekko.util.FutureConverters},
+ * {@link org.apache.pekko.util.JavaDurationConverters} and {@link
+ * org.apache.pekko.util.OptionConverters} for use within Java can be compiled from with Java
+ * sources. This is because methods marked with the Scala 3 inline keyword cannot be called from
+ * within Java (see https://github.com/lampepfl/dotty/issues/19346)
+ */
+public class JavaConverterScala3InlineTest {
+ public void Test() {
Review Comment:
@Test
##########
actor-tests/src/test/java/org/apache/pekko/util/JavaConverterScala3InlineTest.java:
##########
@@ -0,0 +1,39 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * license agreements; and to You under the Apache License, version 2.0:
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * This file is part of the Apache Pekko project, which was derived from Akka.
+ */
+
+package org.apache.pekko.util;
+
+import java.time.Duration;
+import java.util.*;
+import java.util.concurrent.CompletableFuture;
+
+import scala.Option;
+import scala.concurrent.Future;
+
+/**
+ * These tests are here to ensure that methods from {@link org.apache.pekko.util.FutureConverters},
+ * {@link org.apache.pekko.util.JavaDurationConverters} and {@link
+ * org.apache.pekko.util.OptionConverters} for use within Java can be compiled from with Java
+ * sources. This is because methods marked with the Scala 3 inline keyword cannot be called from
+ * within Java (see https://github.com/lampepfl/dotty/issues/19346)
+ */
+public class JavaConverterScala3InlineTest {
+ public void Test() {
+ OptionConverters.toScala(Optional.empty());
+ OptionConverters.toScala(OptionalDouble.of(0));
+ OptionConverters.toScala(OptionalInt.of(0));
+ OptionConverters.toScala(OptionalLong.of(0));
+ OptionConverters.toJava(Option.empty());
+
+ FutureConverters.asJava(Future.successful(""));
+ FutureConverters.asScala(CompletableFuture.completedFuture(""));
+
+ JavaDurationConverters.asFiniteDuration(Duration.ofMillis(0));
Review Comment:
Add Assertions
##########
actor-tests/src/test/java/org/apache/pekko/util/JavaConverterScala3InlineTest.java:
##########
@@ -0,0 +1,39 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * license agreements; and to You under the Apache License, version 2.0:
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * This file is part of the Apache Pekko project, which was derived from Akka.
Review Comment:
If the content is new ,then only asf header
--
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
Re: [PR] Remove Scala 3 inline keyword from Java specific conversion methods [incubator-pekko]
Posted by "mdedetrich (via GitHub)" <gi...@apache.org>.
mdedetrich commented on code in PR #894:
URL: https://github.com/apache/incubator-pekko/pull/894#discussion_r1438780792
##########
actor/src/main/scala/org/apache/pekko/actor/ActorSelection.scala:
##########
@@ -108,8 +108,10 @@ abstract class ActorSelection extends Serializable {
* supplied `timeout`.
*/
@deprecated("Use the overloaded method resolveOne which accepts java.time.Duration instead.", since = "Akka 2.5.20")
- def resolveOneCS(timeout: FiniteDuration): CompletionStage[ActorRef] =
- FutureConverters.asJava[ActorRef](resolveOne(timeout))
+ def resolveOneCS(timeout: FiniteDuration): CompletionStage[ActorRef] = {
+ import FutureConverters._
+ resolveOne(timeout).asJava
Review Comment:
> this changs seems not related with this pr.
It is, specifically this part from the PR description
> Going forward what this means is that if any of the code within Pekko projects happen to call these conversion methods, if the method is being called within a *.scala source then it should use the extension method versions (i.e. the methods defined within implicit final class) in order to get the inline benefits when compiled with Scala 3. Thats the best that can be done in this situation and it also explains the rest of the changes in this PR.
> The API here should be shown to Java programmers, and I think the original way is easier for Java programmers to understand.
This change has zero effect on Java code calling `def resolveOneCS(timeout: FiniteDuration): CompletionStage[ActorRef]`, the inlining is only ocurring in the internal implementation (if thats what you are were implying in your question).
--
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
Re: [PR] Remove Scala 3 inline keyword from Java specific conversion methods [incubator-pekko]
Posted by "He-Pin (via GitHub)" <gi...@apache.org>.
He-Pin commented on code in PR #894:
URL: https://github.com/apache/incubator-pekko/pull/894#discussion_r1438806123
##########
actor-tests/src/test/java/org/apache/pekko/util/JavaConverterScala3InlineTest.java:
##########
@@ -0,0 +1,39 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * license agreements; and to You under the Apache License, version 2.0:
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * This file is part of the Apache Pekko project, which was derived from Akka.
Review Comment:
A copy should keep the lightbend header
--
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
Re: [PR] Remove Scala 3 inline keyword from Java specific conversion methods [incubator-pekko]
Posted by "mdedetrich (via GitHub)" <gi...@apache.org>.
mdedetrich commented on PR #894:
URL: https://github.com/apache/incubator-pekko/pull/894#issuecomment-1873043774
I have updated the PR, fixing the header for `JavaConverterScala3InlineTest` and renaming the method to `compileTest` (so that its also clear that the point of the test is to see that the code compiles, not the actual runtime behaviour of the code being executed).
--
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
Re: [PR] Remove Scala 3 inline keyword from Java specific conversion methods [incubator-pekko]
Posted by "mdedetrich (via GitHub)" <gi...@apache.org>.
mdedetrich commented on code in PR #894:
URL: https://github.com/apache/incubator-pekko/pull/894#discussion_r1438806304
##########
actor-tests/src/test/java/org/apache/pekko/util/JavaConverterScala3InlineTest.java:
##########
@@ -0,0 +1,39 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * license agreements; and to You under the Apache License, version 2.0:
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * This file is part of the Apache Pekko project, which was derived from Akka.
Review Comment:
This isn't a copy, it's a new source file
--
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
Re: [PR] Remove Scala 3 inline keyword from Java specific conversion methods [incubator-pekko]
Posted by "mdedetrich (via GitHub)" <gi...@apache.org>.
mdedetrich commented on code in PR #894:
URL: https://github.com/apache/incubator-pekko/pull/894#discussion_r1438780792
##########
actor/src/main/scala/org/apache/pekko/actor/ActorSelection.scala:
##########
@@ -108,8 +108,10 @@ abstract class ActorSelection extends Serializable {
* supplied `timeout`.
*/
@deprecated("Use the overloaded method resolveOne which accepts java.time.Duration instead.", since = "Akka 2.5.20")
- def resolveOneCS(timeout: FiniteDuration): CompletionStage[ActorRef] =
- FutureConverters.asJava[ActorRef](resolveOne(timeout))
+ def resolveOneCS(timeout: FiniteDuration): CompletionStage[ActorRef] = {
+ import FutureConverters._
+ resolveOne(timeout).asJava
Review Comment:
> this changs seems not related with this pr.
It is, specifically this part from the PR description
> Going forward what this means is that if any of the code within Pekko projects happen to call these conversion methods, if the method is being called within a *.scala source then it should use the extension method versions (i.e. the methods defined within implicit final class) in order to get the inline benefits when compiled with Scala 3. Thats the best that can be done in this situation and it also explains the rest of the changes in this PR.
> The API here should be shown to Java programmers, and I think the original way is easier for Java programmers to understand.
This change has zero effect on Java code calling `def resolveOneCS(timeout: FiniteDuration): CompletionStage[ActorRef]`, the inlining is only ocurring in the internal implementation (if thats what you are were implying in your question).
##########
actor/src/main/scala/org/apache/pekko/actor/ActorSelection.scala:
##########
@@ -108,8 +108,10 @@ abstract class ActorSelection extends Serializable {
* supplied `timeout`.
*/
@deprecated("Use the overloaded method resolveOne which accepts java.time.Duration instead.", since = "Akka 2.5.20")
- def resolveOneCS(timeout: FiniteDuration): CompletionStage[ActorRef] =
- FutureConverters.asJava[ActorRef](resolveOne(timeout))
+ def resolveOneCS(timeout: FiniteDuration): CompletionStage[ActorRef] = {
+ import FutureConverters._
+ resolveOne(timeout).asJava
Review Comment:
> this changs seems not related with this pr.
It is, specifically this part from the PR description
> Going forward what this means is that if any of the code within Pekko projects happen to call these conversion methods, if the method is being called within a *.scala source then it should use the extension method versions (i.e. the methods defined within implicit final class) in order to get the inline benefits when compiled with Scala 3. Thats the best that can be done in this situation and it also explains the rest of the changes in this PR.
Also
> The API here should be shown to Java programmers, and I think the original way is easier for Java programmers to understand.
This change has zero effect on Java code calling `def resolveOneCS(timeout: FiniteDuration): CompletionStage[ActorRef]`, the inlining is only ocurring in the internal implementation (if thats what you are were implying in your question).
--
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
Re: [PR] Remove Scala 3 inline keyword from Java specific conversion methods [incubator-pekko]
Posted by "mdedetrich (via GitHub)" <gi...@apache.org>.
mdedetrich commented on code in PR #894:
URL: https://github.com/apache/incubator-pekko/pull/894#discussion_r1438808197
##########
actor-tests/src/test/java/org/apache/pekko/util/JavaConverterScala3InlineTest.java:
##########
@@ -0,0 +1,39 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * license agreements; and to You under the Apache License, version 2.0:
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * This file is part of the Apache Pekko project, which was derived from Akka.
+ */
+
+package org.apache.pekko.util;
+
+import java.time.Duration;
+import java.util.*;
+import java.util.concurrent.CompletableFuture;
+
+import scala.Option;
+import scala.concurrent.Future;
+
+/**
+ * These tests are here to ensure that methods from {@link org.apache.pekko.util.FutureConverters},
+ * {@link org.apache.pekko.util.JavaDurationConverters} and {@link
+ * org.apache.pekko.util.OptionConverters} for use within Java can be compiled from with Java
+ * sources. This is because methods marked with the Scala 3 inline keyword cannot be called from
+ * within Java (see https://github.com/lampepfl/dotty/issues/19346)
+ */
+public class JavaConverterScala3InlineTest {
+ public void Test() {
Review Comment:
The point of the rest isn't to validate that the methods are correct but rather that the `*.java` compiles.
--
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
Re: [PR] Remove Scala 3 inline keyword from Java specific conversion methods [incubator-pekko]
Posted by "mdedetrich (via GitHub)" <gi...@apache.org>.
mdedetrich commented on PR #894:
URL: https://github.com/apache/incubator-pekko/pull/894#issuecomment-1873077852
> pravega connector compile of a Java class fails with Scala 3
Indeed this appears to be related, merging now
--
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