search

monitorjbl / excel-streaming-reader

An easy-to-use implementation of a streaming Excel reader using Apache POI
Apache License 2.0
960 stars 345 forks source link

support jdk11 builds for testing (needs newer javadoc plugin) #163

Closed pjfanning closed 4 years ago

monitorjbl commented 6 years ago

Being able to build for JDK11 is great, but there's more that's required for (well-formed) libraries. We need to be able to define the module exports in a module-info.java file. We also should keep in mind that this library is definitely going to be used by folks that are stuck on Java 8 for potentially years. Until POI jumps up to JDK11, we need to be able to support them as well.

I think we need publish two Maven artifacts (one for java8 and one for Java11+) and take care not to use non-Java8 compatible features of the language in the code. I opened #164 for this, I think we should talk more about this there.

pjfanning commented 6 years ago

It's not uncommon to test java projects with multiple jdks to check for compile issues and runtime behaviour differences. Since the same code passes in both jdk8 and jdk11 build, I would say that releases should be done from the jdk8 build.

monitorjbl commented 6 years ago

That's a good point, it can't hurt to test that out on the build.

pjfanning commented 6 years ago

I'm not sure why shippable jdk11 build is failing. It did pass once. I use travis on my forked build and it has little trouble with the jdk11 testing. Regarding module-info changes that could be added for better JDK 9+ support, the Apache POI team have decided not to proceed with that in order to have just one set of jars that support Java 8+. I haven't seen too many open source projects that have seen the need to add module-info.

monitorjbl commented 6 years ago

When you first opened this PR, I was under the impression you wanted the library published with Java11 builds. I realize that's not the case now. At some point (probably years from now), POI will jump to Java11 and then it will be necessary.

Even if we were to do this now, the module-info.java is not a hugely important thing because this library's dependencies are mostly Java8-only and the transitives will pull in everything. However, the module system is something that will be important for Java11 builds. In fact, it's basically the only reason to ever produce a separate Java11 artifact because backwards-compatibility keeps Java8 builds working just fine in Java11. If a Java11 build gets published, it should definitely at least define a module-info.java for consumers.