search

jakartaee / inject

Apache License 2.0
17 stars 17 forks source link

Compile to Java 11 with module-info #21

Closed rbygrave closed 3 years ago

rbygrave commented 3 years ago

Removes Automatic-Module-Name manifest entry and replaces it with module-info

Ladicek commented 3 years ago

I'm looking at https://eclipse-ee4j.github.io/jakartaee-platform/jakartaee10/JakartaEE10ReleasePlan, specifically this part:

If a component Specification is planning a Major or Minor version update for Jakarta EE 10, then the recommendation would be to recompile and distribute the specification’s APIs at the Java SE 11 source and target levels. This is not a requirement since there may be reasons to continue compiling and distributing the APIs at an earlier Java source or target level.

There was also a discussion about this on the jakartaee-platform-dev list (which I don't follow), see e.g. https://www.eclipse.org/lists/jakartaee-platform-dev/msg02723.html

My understanding is that we don't intend to do any changes in Jakarta Inject, so I guess it would be best to stay on 8 and just add a generated module-info.class. I'm not sure if the JAR needs to become a MR JAR (as proposed by #20) -- I think Moditect can add the module-info.class directly to the root of the JAR.

Would that make sense? (Genuine question. There's too many things I don't really understand here :-) )

rbygrave commented 3 years ago

I think Moditect can add the module-info.class directly to the root of the JAR.

You are correct, it can. I have modified #20 such that it isn't a multi-version jar.

recommendation ... at the Java SE 11 ... so I guess it would be best to stay on 8

I don't mind (or don't get a vote per say) but I'd also suggest we could do both? That is, release a version 2.0.1 that compiles using Java 8 (with module-info via moditech) and then additionally release a 3.0.0 version using Java 11? In that I think it's going to be nicer longer term to maintain Java 11 without the moditech plugin etc and anyone stuck on java 8 would have the 2.0.0 and 2.0.1 artifacts available to them.

Emily-Jiang commented 3 years ago

I had a second thought on this. Since CDI depends on this, if CDI wants to support Java 8, this spec should compile on Java 8 as well. Otherwise, this will force CDI to move up to Java 11. We should discuss this at CDI spec @Ladicek before we can merge this PR.

overheadhunter commented 3 years ago

I had a second thought on this. Since CDI depends on this, if CDI wants to support Java 8, this spec should compile on Java 8 as well. Otherwise, this will force CDI to move up to Java 11. We should discuss this at CDI spec @Ladicek before we can merge this PR.

hence merge & release #20 before this one, as proposed by @rbygrave in #19

Ladicek commented 3 years ago

Personally, I'd just merge #20, which lets us do a micro release, have a module-info.class, and still target Java 8. I see exactly zero reason to release a major whose only change is the bump to 11.

Emily-Jiang commented 3 years ago

I guess this PR is not needed. Shall we close this PR for now?

rbygrave commented 3 years ago

Yes, I think we can close this for now. I am happy to submit another PR if we want to release a java 11 based artifact.