-
Notifications
You must be signed in to change notification settings - Fork 91
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
Upgrade to Jakarta EE 9 #167
Conversation
1705a0d
to
f801106
Compare
Test with no statements is not useful
private static final Level DEFAULT_LOG_LEVEL = Level.FINEST; | ||
private static final Pattern PAT_SEMICOLON_ENCLOSURE = Pattern.compile("\"(.*?)\""); | ||
private static final Pattern PAT_COMMA = Pattern.compile(","); | ||
private static final transient Level DEFAULT_LOG_LEVEL = Level.FINEST; |
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.
@MarkEWaite Isn't the transient
redundant for static
fields? They should not tamper with serialization?
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.
@MarkEWaite Isn't the
transient
redundant forstatic
fields? They should not tamper with serialization?
I've had bad experiences with the git plugin because I changed things related to serialization. Keeping this code unmodified is more about me not wanting to take risks on a plugin that I do not actively use.
StaplerRequest.fromStaplerRequest2(req), | ||
StaplerResponse.fromStaplerResponse2(rsp), |
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.
We want to avoid calls to fromStaplerRequest2
whenever possible because they will prevent future removal of the compatibility layer. In this case, the ChartUtil.generateGraph was deprecated long ago. Current instructions say that as of 1.320 Bind Graph to the URL space. See hudson.tasks.junit.History as an example (note that doing so involves a bit of URL structure change.)
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.
Before settling with fromStaplerRequest2
I tried to make sense of the deprecation note on ChartUtil.generateGraph
. Looking at hudson.tasks.junit.History
it seems to have been rewritten and refactored quite a lot, to a point where I gave up since I could not find out where to start. If you have any pointers or other examples where I could find a good migration path I‘d be happy to give it another try.
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.
Your experience matches with mine. I didn't spend much time searching, but the time that I spent showed no examples that I could reference or hints for the migration. We may just need to accept this one until more research can be done to find the migration path.
private static final Level DEFAULT_LOG_LEVEL = Level.FINEST; | ||
private static final Pattern PAT_SEMICOLON_ENCLOSURE = Pattern.compile("\"(.*?)\""); | ||
private static final Pattern PAT_COMMA = Pattern.compile(","); | ||
private static final transient Level DEFAULT_LOG_LEVEL = Level.FINEST; |
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.
@MarkEWaite Isn't the
transient
redundant forstatic
fields? They should not tamper with serialization?
I've had bad experiences with the git plugin because I changed things related to serialization. Keeping this code unmodified is more about me not wanting to take risks on a plugin that I do not actively use.
Require Jenkins 2.479.1 and Jakarta EE 9
Jenkins 2.479.1 provides Jakarta EE 9, Eclipse Jetty 12, Spring Security 6, and Java 17.
Testing done
Confirmed that tests pass.
Submitter checklist