testcontainers / testcontainers-jooq-codegen-maven-plugin

jOOQ code generator using Testcontainers
Apache License 2.0
60 stars 10 forks source link

Java 11 #23

Open Donutellko opened 1 year ago

Donutellko commented 1 year ago

Hi! I'm very thankful for the plugin. But unfortunately, I'm not able to use it with Java 11. However, as I see, it's pretty easy to downgrade it to Java 11. Could you explain, what is the reasoning to use Java 17 in this plugin?

And if there is no blocking issue regarding it, could you kindly merge the pull request #22

sivaprasadreddy commented 1 year ago

Actually no reason to make Java 17 as minimum. May be it's good to make Java 11+ compatible.

Can you please update the PR to build with both Java 11 and 17 using GitHub Actions?

lukaseder commented 1 year ago

The jOOQ Open Source Edition's baseline is Java 17 since jOOQ 3.17: https://www.jooq.org/download/versions/#oss

I hope this helps

sivaprasadreddy commented 1 year ago

Oh missed to check the compatibility with the underlying jOOQ generator.

So, if we compile the plugin with Java 11 then they should be able to use the plugin with Java 11 or 17 based on the jOOQ version they want to use.

For ex: Java 11 + jOOQ 3.0, Java 17 + jOOQ 3.17+ should work.

Is my assumption correct?

Donutellko commented 1 year ago

Thank you for your response!

Yes, had to downgrade jOOQ to 3.16.20. I believe that users will be able to update jOOQ version accordingly. Please take a look at PR #22. I'm not sure it will work, as I don't really have a lot of experience with GitHub Actions

Note: I wrote this comment without reading the previous one.

Donutellko commented 1 year ago

So, if we compile the plugin with Java 11 then they should be able to use the plugin with Java 11 or 17 based on the jOOQ version they want to use.

Honestly, I'm not sure what you mean. In my understanding, we don't have to build the plugin with Java 17. The customer will be able to choose jOOQ-codegen version separately, and use it with the plugin that was compiled with Java 11. Or there should be two separate artifacts, like testcontainers-jooq-codegen-maven and testcontainers-jooq-codegen-maven-java11?

sivaprasadreddy commented 1 year ago

I mean we will have only one plugin artifact which will use Java 11. Then people should be able to use the plugin with either Java 11 or 17. Only thing is if they want to use jOOQ 3.17+ they should configure java version to be Java 17+.

lukaseder commented 1 year ago

Is my assumption correct?

Well, jOOQ's commercial distributions all still support Java 8, too. It depends what you want to do with this plugin...

Donutellko commented 1 year ago

So, what is the plan?

sivaprasadreddy commented 1 year ago

In Java most of the time jOOQ is used along with one of the main frameworks like Spring Boot, Quarkus and Micronaut. The latest versions of these frameworks made Java 17 as baseline which is one of the reason to use Java 17 as baseline for this plugin.

I understand that many applications might not have migrated to these latest versions that require Java 17 and still being run on Java 11.

However, to support Java 11+ for this plugin we will have more maintenance work such as:

@Donutellko If you are interested to contribute these changes, I have no objection to add Java 11 support.

@romchellis As you are one of the key contributor, I would like to know your opinion on this?

romchellis commented 1 year ago

In my opinion, with these tradeoffs, I would suggest not downgrading this plugin. It is too costly to reject the latest versions of the underlying dependencies. Another option could be to create another artefact, such as testcontainers-jooq-codegen-maven-plugin-java-11, so that if @Donutellko desires, somebody can maintain the second version of this plugin.

Donutellko commented 1 year ago

Yeah, I'm actually interested in getting experience in maintaining something.

And in my opinion, it might make sense to name it differently, not -java-11. So that it would be possible to also support Java 8 within the same plugin, without creating several separate repositories. Like,testcontainers-jooq-codegen-maven-plugin-java-lts

romchellis commented 1 year ago

Yeah, I'm actually interested in getting experience in maintaining something.

And in my opinion, it might make sense to name it differently, not -java-11. So that it would be possible to also support Java 8 within the same plugin, without creating several separate repositories. Like,testcontainers-jooq-codegen-maven-plugin-java-lts

It's a convention. Typically main artefacts have no prefixes and supporting ones have them. For instance:

<dependency>
    <groupId>org.jooq</groupId>
    <artifactId>jool-java-8</artifactId>
</dependency>
<dependency>
    <groupId>org.apache.commons</groupId>
    <artifactId>commons-lang3</artifactId>
</dependency>
<dependency>
    <groupId>org.apache.commons</groupId>
    <artifactId>commons-lang2</artifactId>
</dependency>
Donutellko commented 1 year ago

Ok, agree