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

Update java version #181

Merged
merged 4 commits into from
Feb 2, 2024
Merged

Update java version #181

merged 4 commits into from
Feb 2, 2024

Conversation

ErikaSignal
Copy link
Contributor

Update Java version to 21 and some of the methods to newer methods that also looks cleaner.

@ErikaSignal ErikaSignal added the java Pull requests that update Java code label Feb 2, 2024
cmatlak
cmatlak previously approved these changes Feb 2, 2024
Copy link
Contributor

@cmatlak cmatlak left a comment

Choose a reason for hiding this comment

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

Looks good and clean, easier to read

@kappsegla
Copy link
Contributor

SonarCloud build looks to fail for some reason. Must be the file sonar.yml in workflows that have some problems. Might be because the actual project on sonarcloud.io seams to be removed. Maybe I did that or the project is too old. Might have to create a new project there and do some changes for it to work.
But the failing looks like its using a too old version of Jacoco plugin that doesn't support java 21?
java.lang.instrument.IllegalClassFormatException: Error while instrumenting org/fungover/haze/HazeDatabaseTest with JaCoCo 0.8.8.202204050719/5dcf34a.

Might have to update pom.xml to use a newer version?

Copy link
Contributor

@robinalfengard robinalfengard left a comment

Choose a reason for hiding this comment

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

Good update, looks a lot cleaner!

But there seems to be a problem with the build? I think the jacoco version in the project does not support java 21. Maybe it needs to be updated in the pom.xml to a newer version.
https://www.jacoco.org/jacoco/trunk/doc/changes.html

@kappsegla
Copy link
Contributor

Should also update java version in Dockerfile, since that is using 19-jre now.
FROM eclipse-temurin:19-jre-jammy

Can be done in this pullrequest but make sure to try to create a docker image locally and run it to make sure everything works. The dockerfile is used by our automatic release process so it must work. :)

The commands to build the image locally should be:

mvn verify
docker build --tag tag:version .

Copy link

sonarcloud bot commented Feb 2, 2024

@ErikaSignal
Copy link
Contributor Author

Thank you for the feedback! Updated the docker version now too and all is checked :D

Copy link
Contributor

@robinalfengard robinalfengard left a comment

Choose a reason for hiding this comment

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

Updated jacoco version fixed previous issue :)

Copy link
Contributor

@cmatlak cmatlak left a comment

Choose a reason for hiding this comment

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

looks good with the version changes, all files seems to build succesfuly

@kappsegla kappsegla added this pull request to the merge queue Feb 2, 2024
Merged via the queue into main with commit b0a94d4 Feb 2, 2024
3 checks passed
@kappsegla kappsegla deleted the update-java branch February 2, 2024 15:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
java Pull requests that update Java code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants