asciidoctor / asciidoclet

:clipboard: A Javadoc Doclet based on Asciidoctor that lets you write Javadoc in the AsciiDoc syntax.
https://github.com/asciidoctor/asciidoclet
Apache License 2.0
133 stars 39 forks source link

Support Java 17 and Java 21 #122

Closed abelsromero closed 6 months ago

abelsromero commented 6 months ago

Currently, the main branch supports Java 11 only. Regardless of the official LTS lifecycle, in practice, Java 11 is still used and should support it, and on top of that for v2.0.0 release we need to ensure Java 17 and 21 LTS also work correctly.

dakusui commented 6 months ago

Hi! Thank you for great Doclet I love and all efforts to support modern JDKs! But supporting Java 17, Java 21, and Java 11 by a single code base is really possible? It seems an abstract method getType(DocTreePath) is added to com.sun.source.util.DocTrees class after Java11. So, supporting Java 17 and Java 21 needs to be done in a separate branch from a support for Java 11, in my opinion.

abelsromero commented 6 months ago

But supporting Java 17, Java 21, and Java 11 by a single code base is really possible?

I don't know yet. Right now in test branch I am focused on identifying required changes, but it seems it won't be possible as you say (based on the failed CI).

If this is not possible, my goal is to find a solution that simplifies the codebase. I hope https://www.baeldung.com/java-multi-release-jar can help with that. If not, then we'll check options, in this order, multi-module build, then branches (v2 for Java 11, then v3 to Java17+).

dakusui commented 6 months ago

Wonderful! Actually, I was doing the same for the rendering part and I opened this pull request. https://github.com/asciidoctor/asciidoclet/pull/124

With this change, we can render JavaDoc using asciidoc on Java 17 + Java 21. I hope this helps.

The rendered result is found here.

https://dakusui.github.io/asciidoclet-2/

I will be cleaning it up and create notes for issues I noticed during my changes.

abelsromero commented 6 months ago

ありがとう :bow:

I will be cleaning it up and create notes for issues I noticed during my changes.

I'll have time during the weekend to review. I am not going to be picky and it is not my intent to become a full-time maintainer for asciidoclet.

For context, there are conversations about official support for MD https://openjdk.org/jeps/8299906, and it would be nice to present AsciiDoc too. But we need asciidoclet working first.

Said that, if your PR works fine, and helps to get things out quickly, then making branches and doing 2 releases (v2 for java11, v3 for Java17 and 21) makes a lot of sense.

dakusui commented 6 months ago

Thank you for sharing your thought! If it becomes a part of official JDK, it will the best for us. BTW, I sent you a follow on x.com. Could you follow me back so that we can discuss details and other things, if you don't mind?

abelsromero commented 6 months ago

I haven't reviewed the PR yet, but after playing with multi-release I am not convinced about it, so, multiple branches it is.

The main issue is that there's no good support for testing at both CLI and IntelliJ (at least in Maven). Even when the JAR works fine and the correct .class are used depending on the Java version, the test run on the default code, so it's not possible to properly test each version of the code (not easily at least). While branches means duplicating code, it much cleaner and CI friendly imo.

@dakusui I did the follow-back, but I'd rather keep the conversation in the issue or the same PR. I don't see much reason to keep things hidden unless there's some private concern you want to address, for which I am open.

dakusui commented 6 months ago

Thanks @abelsromero ,

I agree with discussing the jdk 17/21 supporting topic within the PR/ticket. Aside from that I have a few things that I noticed during my try and not sure I should open tickets for them and that's why I wanted to have a talk before cluttering your ticket space.

About the multi-release thing, I'd agree your opinion and this time, it seems we don't need it to support Java 11/17/21 at the same time as I described in the PR.

Please check it out.