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

Addressed Java 17 compile warnings #769

Merged
merged 1 commit into from Sep 16, 2023
Merged

Addressed Java 17 compile warnings #769

merged 1 commit into from Sep 16, 2023

Conversation

javadev
Copy link
Contributor

@javadev javadev commented Sep 8, 2023

No description provided.

@stleary
Copy link
Owner

stleary commented Sep 8, 2023

@javadev Thanks for taking care of this, will review later today.

Copy link
Contributor

@johnjaylward johnjaylward left a comment

Choose a reason for hiding this comment

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

It looks like there is a warning about the bootstrap not being set properly.

Source Main:

image

Source Test:

image

Can you try a few option in the Maven configuration to see about adding the new "release" flag?

I found this StackOverflow relating to the warning: https://stackoverflow.com/questions/59097317/warning-options-bootstrap-class-path-not-set-in-conjunction-with-source-8

@johnjaylward
Copy link
Contributor

It looks like there is a warning about the bootstrap not being set properly.

Do note that this may be a non-issue as the warning is relating to accidentally compiling with a newer version of the JDK, thus accidentally using new APIs that may not be available in the old JDK being targeted. Since our build pipeline is cross compiling with explicit versions of the JDK that we support, this should NOT be an issue as the JDK 8 part of the pipeline would fail if someone accidentally used an API that wasn't available in JDK 8.

Point being, If this isn't easily fixable, I would not worry about it.

@javadev
Copy link
Contributor Author

javadev commented Sep 11, 2023

I have reviewed the logs, and it appears that these warnings are specific to the Java compilers 11 and 17. It may be advisable to consider creating alternative pom.xml files for these compilers and specifying Java 11 or 17 accordingly.

@javadev
Copy link
Contributor Author

javadev commented Sep 13, 2023

@stleary

The pull request is prepared for merging.

Copy link
Contributor

@johnjaylward johnjaylward left a comment

Choose a reason for hiding this comment

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

Thanks for looking into it. At this time I don't think multiple POM are needed based on the documentation for the warning.

@stleary stleary changed the title Addressed compile warnings Addressed Java 17 compile warnings Sep 13, 2023
@stleary
Copy link
Owner

stleary commented Sep 13, 2023

What problem does this code solve?
Fixes unit test compiler warnings when building with Java 17 (the pipeline does this)

Risks
Low - this only affects the unit tests

Changes to the API?
N/A

Will this require a new release?
No

Should the documentation be updated?
No

Does it break the unit tests?
No

Was any code refactored in this commit?
N/A

Review status
APPROVED

Starting 3-day comment window

@javadev
Copy link
Contributor Author

javadev commented Sep 16, 2023

stleary commented 3 days ago

@stleary stleary merged commit 01727fd into stleary:master Sep 16, 2023
5 checks passed
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

3 participants