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

Updated to Jakarta JAXB and Java 17 (ref: #1075) #1076

Merged
merged 3 commits into from Jan 5, 2024

Conversation

hilbertglm
Copy link
Contributor

I changed the version in the POM to v8, since another issue indicated that the change would be a version change. Other than that, I just used the Intellij IDEA Refactor/Migrate... to change the package names and updated the POMs accordingly. No other functional code changes were made. The unit tests passed.

@emckee2006
Copy link
Contributor

emckee2006 commented Dec 29, 2023 via email

@hilbertglm
Copy link
Contributor Author

I didn't test it, because I assumed it would be a major release branch. I ran it on Java 19 with a target level of 17. I can do it when I get back to the office on Tuesday. Note that I also changed the release in the POM. If you want to reject the pull. I can change the version back to 7.0.3-SNAPSHOT, and test against Java 11, and create a new pull request. That might be the better way.

@hilbertglm
Copy link
Contributor Author

I installed JDK on my local machine and ran mvn clean test install and that completed with no errors. I changed the version back to 7.0.3-SNAPSHOT and the Java version from 17 to 11 in the POM.

Copy link
Contributor

@josemduarte josemduarte left a comment

Choose a reason for hiding this comment

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

Thanks so much. Jaxb has been quite a headache as new versions of java rolled out. Hopefully this should give us some time to breathe.

See comments about pom.

<version>2.3.1</version>
<groupId>jakarta.xml.bind</groupId>
<artifactId>jakarta.xml.bind-api</artifactId>
<version>4.0.0</version>
Copy link
Contributor

Choose a reason for hiding this comment

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

I think versions should be declared in this top pom.xml only (note this is the dependencyManagement section) and not in the modules. Then we have centralised version management.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed explicit versions and scope from POM as required.

<artifactId>jaxb-api</artifactId>
<groupId>jakarta.xml.bind</groupId>
<artifactId>jakarta.xml.bind-api</artifactId>
<version>4.0.0</version>
Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove version here. It should be only in dependencyManagement in top pom.xml

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed explicit versions and scope from POM as required.

</dependency>
<dependency>
<groupId>org.glassfish.jaxb</groupId>
<artifactId>jaxb-runtime</artifactId>
<version>4.0.3</version>
<scope>runtime</scope>
Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove version and scope here. It should be only in dependencyManagement in top pom.xml

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed explicit versions and scope from POM as required.

<artifactId>jaxb-api</artifactId>
<groupId>jakarta.xml.bind</groupId>
<artifactId>jakarta.xml.bind-api</artifactId>
<version>4.0.0</version>
Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove version here. It should be only in dependencyManagement in top pom.xml

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed explicit versions and scope from POM as required.

</dependency>
<dependency>
<groupId>org.glassfish.jaxb</groupId>
<artifactId>jaxb-runtime</artifactId>
<version>4.0.3</version>
<scope>runtime</scope>
Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove version and scope here. It should be only in dependencyManagement in top pom.xml

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed explicit versions and scope from POM as required.

</dependency>
<dependency>
<groupId>org.glassfish.jaxb</groupId>
<artifactId>jaxb-runtime</artifactId>
<version>4.0.3</version>
<scope>runtime</scope>
Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove version and scope here. It should be only in dependencyManagement in top pom.xml

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed explicit versions and scope from POM as required.

<artifactId>jaxb-api</artifactId>
<groupId>jakarta.xml.bind</groupId>
<artifactId>jakarta.xml.bind-api</artifactId>
<version>4.0.0</version>
Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove version here. It should be only in dependencyManagement in top pom.xml

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed explicit versions and scope from POM as required.

</dependency>
<dependency>
<groupId>org.glassfish.jaxb</groupId>
<artifactId>jaxb-runtime</artifactId>
<version>4.0.3</version>
<scope>runtime</scope>
Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove version and scope here. It should be only in dependencyManagement in top pom.xml

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed explicit versions and scope from POM as required.

<artifactId>jaxb-api</artifactId>
<groupId>jakarta.xml.bind</groupId>
<artifactId>jakarta.xml.bind-api</artifactId>
<version>4.0.0</version>
Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove version here. It should be only in dependencyManagement in top pom.xml

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed explicit versions and scope from POM as required.

</dependency>
<dependency>
<groupId>org.glassfish.jaxb</groupId>
<artifactId>jaxb-runtime</artifactId>
<version>4.0.3</version>
<scope>runtime</scope>
Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove version and scope here. It should be only in dependencyManagement in top pom.xml

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed explicit versions and scope from POM as required.

… dependencyManagement section in the POM. Reran unit tests with no errors.
@hilbertglm
Copy link
Contributor Author

Removed explicit scope and version, except in the dependency management section of the POM. Re-compiled and ran unit tests with no error.

@hilbertglm
Copy link
Contributor Author

It appears that openchart was removed from the JBoss Maven repository. I looked and it isn't on Maven Central, either. I Googled around and can't seem to find any mention of it.

@hilbertglm
Copy link
Contributor Author

I took a look at my local repository, and openchart was there on May 26, 2023 when I first compiled biojava. I looked inside the jar, and it was compiled November 28, 2008, so it appears to not be an active project.

Copy link
Contributor

@josemduarte josemduarte 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 the changes! LGTM

Thanks also for checking the openchart dependency. I was fearing it would disappear one day from jboss repo. Since you've tested already, I think we can merge this for now and see how to get rid of the openchart dependency in another PR.

@josemduarte josemduarte merged commit 05c229c into biojava:master Jan 5, 2024
1 check passed
@hilbertglm hilbertglm deleted the 20231229-jaxb branch January 6, 2024 14:56
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