-
Notifications
You must be signed in to change notification settings - Fork 37
Build as multi release jar #62
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
base: master
Are you sure you want to change the base?
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
- make jakarta and javax annotation modules optional at runtime - update GitHub Action to JDK 11 - added module info in java9 source folder - added executions for java 8 and 9 - removed automatic module - added multi release entry to manifest
Sorry for late reply. We're looking for maintainers: #71 Let me know if you're interesting in maintaining this project. Thank you. |
pom.xml
Outdated
</goals> | ||
<configuration> | ||
<source>1.8</source> | ||
<target>1.8</target> |
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.
Instead of source/target, use release
with 8
.
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.
Its practically identical, but you are right. It's a bit clearer that way.
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 think release is meant to be a bit more robust in terms of ensuring backwards compatibility. I can’t say I actually know the difference other than release is typically used in newer Java versions.
module org.openapitools.jackson.nullable { | ||
requires com.fasterxml.jackson.databind; | ||
requires static jakarta.validation; | ||
requires static java.validation; |
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.
Does this work correctly in projects that only have the javax OR jakarta dependency? This project has both as a provided scope, so I expect tests/compile will pass. However an application/library that depends on this project will typically only have one or the other.
Sorry, I'm not super familiar with the module system.
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 have used this in one project when I first created the PR. That project is now suspended, but it worked with JLink at the time. I only hat the Jakarta namespace in that project.
# Conflicts: # pom.xml
There is a problem with the javadoc plugin. Ill have to take a look at that later. |
# Conflicts: # .github/workflows/maven_release.yml
The JavaDoc problem seems to be fixed now. I still don't really know what the problem was... |
There's the below warning emitted when building locally. I don't really understand anything about it, except that it can't be fixed without using Java 17 for the build step. It might be worth checking what other Jackson modules are doing today with Jackson v2 support.
bndtools/bnd#3514 contains some suggestions on how to just ignore the message. I think it can be left alone, but it might be better to add the ignore message config with a note about fixing it when migrating to Java 17/Jackson 3. Aside from that, if the build passes after the merge conflicts are resolved I'm good with this. |
# Conflicts: # .github/workflows/maven_release.yml # .github/workflows/maven_test.yml
As mentioned in #61 here is the build config to build this as a multi release jar with a
module-info.java
for Java 9+. This allows using this dependency with JLink while maintaining support for Java 8.java9
source folder