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 40 forks source link

Unbundled distribution? #70

Closed rockyallen closed 6 years ago

rockyallen commented 6 years ago

It looks like Guava, JRuby, and AsciidoctorJ are bundled in the jar. Is it possible to have a -core version without them, like asciidoctor-ant has done?

johncarl81 commented 6 years ago

We could do this... is there any use case that this satisfies?

rockyallen commented 6 years ago

Mainly to make it easier to test or use updated libraries without waiting for them to reach asciidoclet. The Uber-jar also causes me problems with download and redistribution file size, but perhaps that is very specific to my setup.

mojavelinux commented 6 years ago

I'd be fine with unbundling and changing these to dependencies. We could also add a dist task that produces an uberjar for people who still want it.

mojavelinux commented 6 years ago

@msgilligan What are your thoughts?

msgilligan commented 6 years ago

As I user, I liked it when the Asciidoctor Gradle plugin allowed me to choose Asciidoc versions separately than that of the Plugin. So in general I support this.

I'm not sure what the the in-depth technical issues are -- I"m new to the codebase.

One thing I would like to do -- and did some research on -- is move the plugin to requiring Java 8 (or 9) and eliminating the Guava dependency. Many of the things it is used for are provided in 8 and 9. This may be part of the rearchitect for the new Java 9 javadoc API.

I'm also hoping to learn the Java 9 modules stuff soon, so that might play a factor in the design of an approach.

One decision we need to make is when and how to draw the line between the pre-Java-9 version and the Java-9+ version.

johncarl81 commented 6 years ago

I'd like to see Guava removed also, but that's probably influenced by my Android work - Guava is known to be a huge library and consumes a ton of the limited method references. Should be relatively easy. Maybe we bundle this into the Java 9 jump?

msgilligan commented 6 years ago

I agree, let's (at least try) to pull out Guava in the Java 9 jump. Java 8 adds Optional which I see is one of the things you're using from Guava. Java 9 I think adds better support for immutable lists which I think one of the other things you're using. So that seems like the right plan.

msgilligan commented 6 years ago

It is possible to upgrade the version of AsciidoctorJ by directly referencing it in a Gradle configuration:

    configurations {
        asciidoclet
    }

    dependencies {
        asciidoclet "org.asciidoctor:asciidoclet:${asciidocletVersion}"
        asciidoclet "org.asciidoctor:asciidoctorj:${asciidoctorjVersion}"
        asciidoclet "org.asciidoctor:asciidoctorj-diagram:${asciidoctorjDiagramVersion}"
    }

And also note that the setting of asciidoctorjDiagramVersion can cause a (perhaps unexpectedly) newer version of AsciidoctorJ to be transitively included.

mojavelinux commented 6 years ago

It is possible to upgrade the version of AsciidoctorJ by directly referencing it in a Gradle configuration:

But this is extremely fragile with AsciidoctorJ being bundled. I'm 1000% behind unbundling. It's absolutely the right thing to do in my mind. It will also make debugging way simpler.

mojavelinux commented 6 years ago

And also note that the setting of asciidoctorjDiagramVersion can cause a (perhaps unexpectedly) newer version of AsciidoctorJ to be transitively included.

That's an issue that should be addressed in AsciidoctorJ Diagram. I don't think it should be dictating the version of AsciidoctorJ.

msgilligan commented 6 years ago

OK, so it sounds like we should prioritize this issue, so that we can also fix #45.

@johncarl81 What would you think of converting the build of Asciidoclet from Maven to Gradle?

(I ask about Gradle, because I know it well and would be much more comfortable maintaining a Gradle build than one in Maven)

msgilligan commented 6 years ago

Is there any use case for continuing to support an "uber jar"? In other words, should we change to releasing only an unbundled distribution?

johncarl81 commented 6 years ago

Supporting a regular (unbundled) and uber jar simultaneously should be relatively simple.

The use case for the uber jar is using the library outside of a dependency-managed build. Being able to download and use the jar directly has some utility to certain users (honestly not myself). Here's the history: https://github.com/asciidoctor/asciidoclet/issues/38 https://github.com/asciidoctor/asciidoclet/pull/43

I'd be up for switching to gradle if it can be shown to be that it is easier to use than Maven. One thing in particular I like about the Maven build is the release plugin. This allows us to issue a mvn release:prepare && mvn release:perform on the library and have the non-SNAPSHOT pushed up to maven central, SNAPSHOT and release versions enforced, and the associated commits issued.

msgilligan commented 6 years ago

has some utility to certain users (honestly not myself). Here's the history: #38 #43

Yeah, that's a legitimate use case, but probably increasingly rare. My vote would be to drop support for it and see if anyone requests it. I'd prefer to keep the build and release as simple as possible.

I'd be up for switching to gradle if it can be shown to be that it is easier to use than Maven.

Well, I figured out what I needed to do to unbundle and submitted a PR. That wasn't hard! I do prefer Gradle, but let's stick with Maven for now -- we've got enough fish to fry. If something comes up where Gradle might help, I'll revisit the suggestion.

One thing in particular I like about the Maven build is the release plugin.

I think there are similar plugins for Gradle, but I haven't used them. Looking at how to do that is on my todo list for other projects. But, yeah, let's stick with Maven for now.

johncarl81 commented 6 years ago

@msgilligan Out of curiosity, what feature of asciidoctorj are you looking to add by upgrading to 1.5.7?

msgilligan commented 6 years ago

what feature of asciidoctorj are you looking to add by upgrading to 1.5.7

Well, I was hoping those annoying "tilt/haml" warnings were going to be removed from my builds. But in general, I just want to track the latest & greatest. Especially for the Java 9+ work.

msgilligan commented 6 years ago

Now that #86 is merged, once we have a release made that includes both the unbundled JAR and the -shaded JAR, we can close this issue.

johncarl81 commented 6 years ago

Released in 1.5.6