Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add the enforcer plugin and require a minimum of Java 21 to compile t… #1203

Closed
wants to merge 2 commits into from

Conversation

jamezp
Copy link
Contributor

@jamezp jamezp commented Dec 13, 2023

The minimum level should remain at Java SE 17. The TCK itself should also require Java SE 17 as a minimum.

resolves #1202

jaxrs-tck/pom.xml Outdated Show resolved Hide resolved
pom.xml Outdated Show resolved Hide resolved
pom.xml Outdated Show resolved Hide resolved
pom.xml Outdated Show resolved Hide resolved
…his project. For the API, compile down to Java 17. Ensure the TCK is compiled to Java 21.

Add a profile for when a src/main/java21 directory is present, the JAR will become a multi-release JAR.

Signed-off-by: James R. Perkins <jperkins@redhat.com>
@jamezp jamezp force-pushed the compile-to-java-17 branch 2 times, most recently from 177e9a2 to 1925da9 Compare December 19, 2023 18:08
…at Java SE 17. The TCK itself should also require Java SE 17 as a minimum.

Signed-off-by: James R. Perkins <jperkins@redhat.com>
@jamezp
Copy link
Contributor Author

jamezp commented Dec 19, 2023

Please note I've changed this PR to have the TCK compile to Java SE 17 as well. It was noted in the Jakarta EE spec meeting there was never an intent for specification TCK's to require Java 21.

@jansupol
Copy link
Contributor

So we can just set release to 17 in the compiler plugin and drop the enforcer plugin?

@jamezp
Copy link
Contributor Author

jamezp commented Dec 19, 2023

So we can just set release to 17 in the compiler plugin and drop the enforcer plugin?

Yes, we could do that as well. The only reason for the enforcer plugin is to know early if you're on the right JDK. It's in no way critical.

Copy link
Contributor

@jansupol jansupol left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Early JDK version check makes sense

@@ -13,7 +13,7 @@ jobs:
runs-on: ubuntu-latest
strategy:
matrix:
java: [ '21-ea' ]
java: [ '17', '21' ]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not what we discussed. It simply runs two parallel builds, one using JDK 17 and one using JDK 21. For a MR, we just need a single build on JDK 21 but with two compilation targets, 17 and 21.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We currently have no reason for a MR JAR. We can consider adding it back, but without the need for a META-INF/versions/21 it makes no sense.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The MR was the last that you and me agreed upon, but indeed, now that Arjan vetoed against the MR, my comment is obsolete.

@jansupol
Copy link
Contributor

Here it says:

For the Jakarta EE Platform (Platform, Web and Core), the Java compiler --release option is 17. For the component specs, the Jakarta EE Platform requires the Java compiler --release option is at most 17, but component specifications can decide on a lower level.

@jamezp
Copy link
Contributor Author

jamezp commented Jan 24, 2024

Given the proposal of 3.2, I'm going to close this. We can re-address it if plans change.

@jamezp jamezp closed this Jan 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants