This repository has been archived by the owner. It is now read-only.

JAXB implementation is a separate set of JAR files with split package issues #1152

Merged
merged 7 commits into from Nov 7, 2017

Conversation

Projects
None yet
5 participants
@zhengjl
Member

zhengjl commented Oct 25, 2017

@zhengjl zhengjl requested review from lukasj and bravehorsie Oct 25, 2017

@bertramn

This comment has been minimized.

Show comment
Hide comment
@bertramn

bertramn Oct 29, 2017

Yes this library can do with some simplification of the package structure, well done!

bertramn commented Oct 29, 2017

Yes this library can do with some simplification of the package structure, well done!

@bravehorsie

Thanks @zhengjl !
Overall it looks good, I have these observations:

Module names for depedencies of jaxb-impl like istack.commons.runtime, txw2, FastInfoset, etc are declared as automatic jar names, for jars copied to module path by dependency-plugin right? It is necessary to modularize it natively as deep as we can / own sources. So in addition we need to add module-info.java to all our subprojects of jaxb.

When jaxb-ri.zip bundle (what we deliver as standalone binary) is built, these classes are shaded / repackaged inside lib/jaxb-*.jar, so you will not have modules like FastInfoset, txw2, etc there. jaxb-ri.bundle may need more elaboration in order for modularization to work.

@lukasj

This comment has been minimized.

Show comment
Hide comment
@lukasj

lukasj Nov 2, 2017

Member

I'd perhaps go with "Automatic-Module-Name" header in manifest first instead of creating module-infos.1 "Add-Exports" manifest header exists as well...

Member

lukasj commented Nov 2, 2017

I'd perhaps go with "Automatic-Module-Name" header in manifest first instead of creating module-infos.1 "Add-Exports" manifest header exists as well...

@m0mus

This comment has been minimized.

Show comment
Hide comment
@m0mus

m0mus Nov 7, 2017

Member

Looks good for me.

Member

m0mus commented Nov 7, 2017

Looks good for me.

@bravehorsie bravehorsie merged commit f54663a into javaee:master Nov 7, 2017

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.