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

[issue172] Migrate to a Jakarta namespaced activation API. #177

Merged
merged 1 commit into from
May 25, 2022

Conversation

jamezp
Copy link
Contributor

@jamezp jamezp commented May 24, 2022

Upstream is #176, however this one is a bit different. #152 seemed to partially solve this, but never changed the package names. I noticed it imports the implementation too instead of the API, which might be required but I'm not sure. The 2.13 won't currently compile without this change.

Let me know if the API vs the implementation is preferred and I can either change this PR or the upstream #176 one.

@cowtowncoder
Copy link
Member

I think API should be preferred, not implementation, assuming that's where JAXB/Jakarta-XML-bind annotations live: module makes no use of actual implementation. Unless unit tests used it? (if so, impl should be test-only dependency).

However... I wonder if there are issues wrt backwards compatibility within minor version, wrt impl vs api.
But the main reason I suspect it's fine is that "Jakarta" variants were only added in 2.13 (as per https://github.com/FasterXML/jackson/wiki/Jackson-Release-2.13) so there isn't much usage; and it is likely that anyone trying to use the module might have hit this same issue.

So I will rely on your expertise @jamezp here: I assume your assessment is correct.

Signed-off-by: James R. Perkins <jperkins@redhat.com>
@jamezp jamezp force-pushed the jakarta-activation-fix-2.13 branch from 97c2e1a to c214a55 Compare May 24, 2022 22:42
@jamezp
Copy link
Contributor Author

jamezp commented May 24, 2022

Okay perfect. I think the API is appropriate here as well. I've updated this PR to use the API and I'll close the 2.14 one. Thank you!

@jamezp
Copy link
Contributor Author

jamezp commented May 24, 2022

resolves #175

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.

2 participants