Skip to content
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

Support java Deprecated on Scala symbols #348

Merged
merged 2 commits into from
Mar 8, 2024
Merged

Conversation

lrytz
Copy link
Contributor

@lrytz lrytz commented Mar 4, 2024

Fixes #347

@lrytz
Copy link
Contributor Author

lrytz commented Mar 4, 2024

I don't seem to understand how these 2.12.x.patch files work.

@lrytz
Copy link
Contributor Author

lrytz commented Mar 4, 2024

Not sure what's going on. When running test using 2.13.12 with the original patch file I see the following output:

diff -wurN -I ^ *// -I ^ *private  java\.lang\.Object readResolve target/expected_output/basic/akka/rk/buh/is/it/DontTouchThis.java /var/folders/8d/8tnybb497mbgq2v08vgcr5c80000gn/T/genjavadoc-java2488064058609746431/akka/rk/buh/is/it/DontTouchThis.java
--- target/expected_output/basic/akka/rk/buh/is/it/DontTouchThis.java	2024-03-04 16:44:23
+++ /var/folders/8d/8tnybb497mbgq2v08vgcr5c80000gn/T/genjavadoc-java2488064058609746431/akka/rk/buh/is/it/DontTouchThis.java	2024-03-04 16:44:23
@@ -12,8 +12,6 @@
   public void alreadyDeprecatedInComment () { throw new RuntimeException(); }
   /**
    * buh!
-   *
-   * @deprecated
    */
   public  void javaDeprecatedThingie ()  { throw new RuntimeException(); }
   /**
[error] Test com.typesafe.genjavadoc.BasicSpec.compileSourcesAndGenerateExpectedOutput failed: java.lang.AssertionError: assertion failed, took 0.509 sec
[error]     at scala.Predef$.assert(Predef.scala:264)
[error]     at com.typesafe.genjavadoc.util.CompilerSpec.$anonfun$lines$2(CompilerSpec.scala:47)

So I added that patch to 2.13.12.patch. Now running test (still on 2.13.12) gives

diff -wurN -I ^ *// -I ^ *private  java\.lang\.Object readResolve target/expected_output/basic/akka/rk/buh/is/it/DontTouchThis.java.orig /var/folders/8d/8tnybb497mbgq2v08vgcr5c80000gn/T/genjavadoc-java10649405226718870908/akka/rk/buh/is/it/DontTouchThis.java.orig
--- target/expected_output/basic/akka/rk/buh/is/it/DontTouchThis.java.orig	2024-03-04 16:46:13
+++ /var/folders/8d/8tnybb497mbgq2v08vgcr5c80000gn/T/genjavadoc-java10649405226718870908/akka/rk/buh/is/it/DontTouchThis.java.orig	1970-01-01 01:00:00
@@ -1,25 +0,0 @@
-package akka.rk.buh.is.it;
-/**
- * Don't touch this!
- *
- * @deprecated This is replaced by TouchThisInstead. Since now.
- */
-public class DontTouchThis {
-  public   DontTouchThis () { throw new RuntimeException(); }
-  /**
-   * @deprecated This is already deprecated. Since now.
-   */
-  public void alreadyDeprecatedInComment () { throw new RuntimeException(); }
-  /**
-   * buh!
-   *
-   * @deprecated
-   */
-  public void javaDeprecatedThingie () { throw new RuntimeException(); }
-  /**
-   * Some methods are forever.
-   *
-   * @deprecated This is replaced by someDiamondsAre. Since now.
-   */
-  public void orNotSoMuch () { throw new RuntimeException(); }
-}
[error] Test com.typesafe.genjavadoc.BasicSpec.compileSourcesAndGenerateExpectedOutput failed: java.lang.AssertionError: assertion failed, took 0.528 sec
[error]     at scala.Predef$.assert(Predef.scala:264)
[error]     at com.typesafe.genjavadoc.util.CompilerSpec.$anonfun$lines$2(CompilerSpec.scala:47)

Avoid leaving behind an '.orig' file which would fail the test
@raboof
Copy link
Contributor

raboof commented Mar 5, 2024

Not sure what's going on. When running test using 2.13.12 with the original patch file I see the following output:

diff -wurN -I ^ *// -I ^ *private  java\.lang\.Object readResolve target/expected_output/basic/akka/rk/buh/is/it/DontTouchThis.java /var/folders/8d/8tnybb497mbgq2v08vgcr5c80000gn/T/genjavadoc-java2488064058609746431/akka/rk/buh/is/it/DontTouchThis.java
--- target/expected_output/basic/akka/rk/buh/is/it/DontTouchThis.java	2024-03-04 16:44:23
+++ /var/folders/8d/8tnybb497mbgq2v08vgcr5c80000gn/T/genjavadoc-java2488064058609746431/akka/rk/buh/is/it/DontTouchThis.java	2024-03-04 16:44:23
@@ -12,8 +12,6 @@
   public void alreadyDeprecatedInComment () { throw new RuntimeException(); }
   /**
    * buh!
-   *
-   * @deprecated
    */
   public  void javaDeprecatedThingie ()  { throw new RuntimeException(); }
   /**
[error] Test com.typesafe.genjavadoc.BasicSpec.compileSourcesAndGenerateExpectedOutput failed: java.lang.AssertionError: assertion failed, took 0.509 sec
[error]     at scala.Predef$.assert(Predef.scala:264)
[error]     at com.typesafe.genjavadoc.util.CompilerSpec.$anonfun$lines$2(CompilerSpec.scala:47)

So I added that patch to 2.13.12.patch. Now running test (still on 2.13.12) gives

diff -wurN -I ^ *// -I ^ *private  java\.lang\.Object readResolve target/expected_output/basic/akka/rk/buh/is/it/DontTouchThis.java.orig /var/folders/8d/8tnybb497mbgq2v08vgcr5c80000gn/T/genjavadoc-java10649405226718870908/akka/rk/buh/is/it/DontTouchThis.java.orig
--- target/expected_output/basic/akka/rk/buh/is/it/DontTouchThis.java.orig	2024-03-04 16:46:13
+++ /var/folders/8d/8tnybb497mbgq2v08vgcr5c80000gn/T/genjavadoc-java10649405226718870908/akka/rk/buh/is/it/DontTouchThis.java.orig	1970-01-01 01:00:00

The patch applied, but with 'fuzz', leaving behind an .orig, which is what you're seeing in this diff. Not sure where the 'fuzz' is coming from, but it seems convenient to allow, so lrytz#1 might help

@lightbend-cla-validator
Copy link
Collaborator

Hi @raboof,

Thank you for your contribution! We really value the time you've taken to put this together.

We see that you have signed the Lightbend Contributors License Agreement before, however, the CLA has changed since you last signed it.
Please review the new CLA and sign it before we proceed with reviewing this pull request:

http://www.lightbend.com/contribute/cla

@lrytz
Copy link
Contributor Author

lrytz commented Mar 5, 2024

Thank you!!!

@lrytz lrytz requested a review from SethTisue March 5, 2024 11:56
@raboof
Copy link
Contributor

raboof commented Mar 5, 2024

We see that you have signed the Lightbend Contributors License Agreement before, however, the CLA has changed since you last signed it.

Is there a diff somewhere?

(the commit is trivial enough that you can also just squash it in, I don't think it's copyrightable)

@SethTisue
Copy link
Member

@JustinPihony the CLA-changed notice is new to me... any idea what that's about?

@SethTisue
Copy link
Member

Is there a diff somewhere?

I'm inquiring internally.

I agree that the change you contributed (thank you!) is too small for a CLA issue to block merging.

@SethTisue SethTisue merged commit 98a14cc into lightbend:main Mar 8, 2024
21 of 22 checks passed
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.

genjavadoc is broken with Scala 2.13.13
4 participants