-
Notifications
You must be signed in to change notification settings - Fork 601
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
JAX-B: Add xmlBinding-4.0 Impl and tests #21863
Conversation
#build |
Please code review feature-related files, @OpenLiberty/delivery-approvers |
Your personal build request is at https://wasrtc.hursley.ibm.com:9443/jazz/resource/itemOid/com.ibm.team.build.BuildResult/_eC0QMAywEe2vacLbpVXjpQ Target locations of links might be accessible only to IBM employees. |
@@ -0,0 +1,65 @@ | |||
/** |
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.
Missing copyright header
@@ -0,0 +1,47 @@ | |||
/** |
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.
missing header
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.
Please fix copyright headers ASAP
@tevans78 done! |
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.
Suggested changes
...a.xmlBinding.4.0/resources/io/openliberty/jakarta/xmlbinding/resources/JAXBMessages.nlsprops
Outdated
Show resolved
Hide resolved
...a.xmlBinding.4.0/resources/io/openliberty/jakarta/xmlbinding/resources/JAXBMessages.nlsprops
Outdated
Show resolved
Hide resolved
...a.xmlBinding.4.0/resources/io/openliberty/jakarta/xmlbinding/resources/JAXBMessages.nlsprops
Outdated
Show resolved
Hide resolved
#build |
Please code review feature-related files, @OpenLiberty/delivery-approvers |
Your personal build request is at https://wasrtc.hursley.ibm.com:9443/jazz/resource/itemOid/com.ibm.team.build.BuildResult/_6KhZIA1dEe2vacLbpVXjpQ Target locations of links might be accessible only to IBM employees. |
@helyarp changes should be incorporated now! Thank you! |
#build |
Please code review feature-related files, @OpenLiberty/delivery-approvers |
Your personal build request is at https://wasrtc.hursley.ibm.com:9443/jazz/resource/itemOid/com.ibm.team.build.BuildResult/_B6gjoA1eEe2vacLbpVXjpQ Target locations of links might be accessible only to IBM employees. |
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 for making the changes
The build neuwerk-21863-20220726-2149 For help analyzing your personal build, go to https://cognitive.hursley.ibm.com/buildAnalysis.html?uuid=_B6gjoA1eEe2vacLbpVXjpQ |
The build neuwerk-21863-20220726-2148 For help analyzing your personal build, go to https://cognitive.hursley.ibm.com/buildAnalysis.html?uuid=_6KhZIA1dEe2vacLbpVXjpQ |
L2 message review completed |
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.
In general I would like to see EE10 only tests to be in a new fat project instead of reusing com.ibm.ws.jaxb_fat. Something like io.openliberty.xmlBinding.4.0.internal_fat.
dev/com.ibm.ws.jaxb_fat/fat/src/com/ibm/ws/jaxb/fat/LibertyJAXBContextTest.java
Outdated
Show resolved
Hide resolved
dev/com.ibm.ws.jaxb_fat/fat/src/com/ibm/ws/jaxb/fat/LibertyJAXBTest.java
Outdated
Show resolved
Hide resolved
dev/com.ibm.ws.jaxb_fat/fat/src/com/ibm/ws/jaxb/fat/LibertyThirdPartyJAXBImplContextTest.java
Outdated
Show resolved
Hide resolved
dev/com.ibm.ws.jaxb_fat/fat/src/com/ibm/ws/jaxb/fat/LibertyJAXBTest.java
Outdated
Show resolved
Hide resolved
dev/io.openliberty.jakarta.xmlBinding.4.0/src/jakarta/xml/bind/ContextFinder.java
Outdated
Show resolved
Hide resolved
e65b14a
to
cd2a8c9
Compare
#build |
Please code review feature-related files, @OpenLiberty/delivery-approvers |
Your personal build request is at https://wasrtc.hursley.ibm.com:9443/jazz/resource/itemOid/com.ibm.team.build.BuildResult/_-PDOIBO2Ee2JSL2ccGwYRA Target locations of links might be accessible only to IBM employees. |
228684f
to
1bd1961
Compare
#build |
Please code review feature-related files, @OpenLiberty/delivery-approvers |
Your personal build request is at https://wasrtc.hursley.ibm.com:9443/jazz/resource/itemOid/com.ibm.team.build.BuildResult/_NHfZthQSEe2JSL2ccGwYRA Target locations of links might be accessible only to IBM employees. |
The build neuwerk-21863-20220803-2349 For help analyzing your personal build, go to https://cognitive.hursley.ibm.com/buildAnalysis.html?uuid=_-PDOIBO2Ee2JSL2ccGwYRA |
1bd1961
to
fe901b4
Compare
#build |
Please code review feature-related files, @OpenLiberty/delivery-approvers |
Your personal build request is at https://wasrtc.hursley.ibm.com:9443/jazz/resource/itemOid/com.ibm.team.build.BuildResult/_0Y8mQBRnEe2JSL2ccGwYRA Target locations of links might be accessible only to IBM employees. |
The build neuwerk-21863-20220804-1035 For help analyzing your personal build, go to https://cognitive.hursley.ibm.com/buildAnalysis.html?uuid=_NHfZthQSEe2JSL2ccGwYRA |
The build neuwerk-21863-20220804-2048 |
#libby |
// This file was generated by the JavaTM Architecture for XML Binding(JAXB) Reference Implementation, vIBM 2.2.3-11/28/2011 06:17 AM(foreman)- | ||
// See <a href="http://java.sun.com/xml/jaxb">http://java.sun.com/xml/jaxb</a> | ||
// Any modifications to this file will be lost upon recompilation of the source schema. | ||
// Generated on: 2013.02.07 at 02:15:17 PM CST |
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 comment can’t be true since this file contains jakarta imports
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.
Good point! I did manually update these, and altered them in other ways so I've amended the commit to add the normal IBM copyright.
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.
Approved. One minor issue was with the comments in the ‘jaxb.web.dataobjects’. They say that the classes were auto generated in 2013, which can’t be true since they include jakarta imports.
fe901b4
to
ea233e5
Compare
#libby |
Code analysis and actionsDO NOT DELETE THIS COMMENT.
|
This PR does a few things:
1.) It integrates the xmlBinding-4.0 impl into open-liberty
2.) It disables a handful of NoClassDefinitionFoundErrors during the JAXBContext look up, to allow for the case of where a non Default implementation is configured, but not available to the TCCL, and rather than failing prints a warning and falls back to the RI for use.
3.) Expands the
com.ibm.ws.jaxb_fat
test bucket to more thoroughly test JAXBContext creation for marshalling and unmarshalling, but also tests the above changers outline in number 2 above