-
Notifications
You must be signed in to change notification settings - Fork 101
New recipe to only add JAXB API dependencies without runtime dependencies #727
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
Conversation
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.
Some suggestions could not be made:
- src/main/resources/META-INF/rewrite/examples.yml
- lines 997-1007
- lines 1036-1042
newVersion: 2.5.x | ||
--- | ||
type: specs.openrewrite.org/v1beta/recipe | ||
name: org.openrewrite.java.migrate.javax.AddJaxbAPIDependencies |
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.
Should we reference this new org.openrewrite.java.migrate.javax.AddJaxbAPIDependencies
from anywhere?
Without that folks would need to seek it out and run it specifically, whereas perhaps it needs to hook into Java 11 upgrade?
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.
The current Java 11 upgrade list adds some jakarta runtime dependencies to the application. That makes sense for some use cases, but it will cause errors for users migrating to an app server which already provides these runtime dependencies. Currently, we are calling some of the Java 11 recipes piecemeal in order to support that managed scenario instead of using the full Java 11 list.
For this recipe, we will use it instead of the existing AddJaxbDependencies
which is already on the Java 11 list.
One potential long term solution is to break up the Java 11 list:
- A list for users migrating to a managed environment
- A list for users migrating to an unmanaged environment (this could remain the default suggestion)
- A list for common recipes to be referenced by both
This would add some additional complexity though.
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.
This has now been implemented with the latest commits here
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.
Could we move the common recipe components into another group, and have the runtime/no-runtime recipe groups call that along with their own runtime addition/removal recipes?
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.
Thanks both for iterating on the right approach here!
What's changed?
A new recipe was added to update the JAXB API dependency when migrating to Jakarta EE 9 and remove any JAXB runtime dependencies.
What's your motivation?
This is necessary when migrating to an app server where the JAXB implementation is provided by the runtime.
Anyone you would like to review specifically?
@timtebeek
Checklist