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

require java.xml directly so that java.desktop must not be available #491

Conversation

tomsontom
Copy link
Contributor

@tomsontom tomsontom commented May 18, 2021

fixes #490

@tomsontom
Copy link
Contributor Author

As outlined in #490 the question is whether it should be require static and the code in DefaultSerializers should gracefully handle the CNF and allow users to even get rid of java.xml dependency if they don't need it.

From a backwards compat point of view there's no difference. Nobody could have used yasson today unless they required java.desktop anyways so adding the java.xml dependency does not alter their runtime image.

@mkarg
Copy link
Member

mkarg commented Jun 8, 2021

@m0mus Dmitry, kindly pinging you. :-)

@tomsontom Please fix the build fail, see failing checks above (wrong copyright year). Thanks.

@tomsontom tomsontom force-pushed the 490_module-info_does_not_require_java.xml_directly branch from 84817bb to 6a502e8 Compare June 9, 2021 10:00
@tomsontom
Copy link
Contributor Author

@Verdent I think I implemented what you suggested in #490
@mkarg I fixed the build issue interesting although is that the checkstyle:checkstyle goal fails locally in a class I haven touched (StrategiesProvider.java) but succeeds on travis

@Verdent Verdent self-requested a review June 9, 2021 11:30
Copy link
Member

@Verdent Verdent left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you :-)

@Verdent Verdent merged commit f084768 into eclipse-ee4j:master Jun 9, 2021
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.

module-info.java does not require java.xml directly
3 participants