-
Notifications
You must be signed in to change notification settings - Fork 53
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
[JENKINS-56312] Prevent adding splits with the wrong JDK #115
[JENKINS-56312] Prevent adding splits with the wrong JDK #115
Conversation
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.
🐝 empty PRs are the best!
Ops, this is a mistake... fixing my PR now, is not going to be much bigger :P |
a7a959d
to
fca59b1
Compare
@Wadeck Now you can review, as promised not much bigger :) |
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 code seems fine. What you call split is a detached plugin, right?
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.
JavaSpecificationVersion should be used for the version comparison here. Some test automation would not hurt as well, but we could deliver a quick fix as is
plugins-compat-tester/src/main/java/org/jenkins/tools/test/PluginCompatTester.java
Outdated
Show resolved
Hide resolved
plugins-compat-tester/src/main/java/org/jenkins/tools/test/PluginCompatTester.java
Outdated
Show resolved
Hide resolved
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.
I agree with @oleg-nenashev about the Java version comparison.
Co-Authored-By: raul-arabaolaza <raul.arabaolaza@gmail.com>
Exactly :) |
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.
LGTM
JENKINS-56312 Only add splits with JDK field if testJDK is newer than the specified JDK version.
This is the first of two fixes to prevent failures related to the PCT adding
jaxb
plugin to builds outside of java 11, second one will make sure the addedjaxb
dependency uses the proper groupId instead oforg.jenkins-ci.plugins