-
Notifications
You must be signed in to change notification settings - Fork 1.5k
[NO-JIRA] Update enforcer-plugin to releasing with JDK 21 #1532
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
base: main
Are you sure you want to change the base?
Conversation
|
The java version rule is just about the JVM running the enforcement, i.e the current runtime. It has nothing to do with the compiler settings or what the build output compiles for / runs on, which is all independent. Wasnt the idea to only require release builds are done with JDK 21+? This change also prevents building testing on 17+, which would ideally still be a thing if supporting/targeting it? |
|
@gemmellr good catch. I'll move this change inside the apache-release profi 8000 le (edited for exact profile name) |
8666755 to
ff3fed9
Compare
|
Output when building with JDK 17 and apache-release profile |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change looks good to me for the enforcer.
However, if we merge it, it means Jenkins will fail. I suggest to update Jenkinsfile as part of this PR.
I dont see mention of it enabling the apache-release profile, so I dont think it will fail unless thats being introduced elsewhere. |
ff3fed9 to
f251a93
Compare
| <rules> | ||
| <requireMavenVersion> | ||
| <version>3.0.5</version> | ||
| <version>3.6.3</version> | 8000
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The parent pom 37 now requires Maven 3.9 to build. Given 3.8.x is now entirely out of support and new plugin releases are aimed at 3.9+, I'd say its time to do the same (and have already done so in other projects). I'd also defer to its enforcer definition (by removing this one entirely) at this point to simplify the pom.
You could also get rid of the java version check below this one (but not the other one in the release profile) and defer to the parents one. They have also defined a new property in v37 to link all the java verison enforcement and compiler settings together, though personally I prefer having the compiler properties set in the components own pom and so am still setting them regardless.
| <execution> | ||
| <id>enforce-maven</id> | ||
| <goals> | ||
| <goal>enforce</goal> | ||
| </goals> | ||
| <configuration> | ||
| <rules> | ||
| <requireMavenVersion> | ||
| <version>3.6.3</version> | ||
| </requireMavenVersion> | ||
| </rules> | ||
| </configuration> | ||
| </execution> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could similarly remove this maven version enforcement.
I tested building with JDK 21 and then running with JDK 17 without a problem.
The docs for enforcer plugin make it read that this setting impacts runtime, but it does not in fact change the compiler-plugin values used for source/target/release.
NOTE: This needs to be merged into main after main is updated to 6.3.0-SNAPSHOT