Hi Lance,
On 02/14/2017 09:28 PM, Lance Andersen wrote:
Hi Roman,
I made a pass through the changes. And here are some general comments
which are mostly minor in nature….
HTH
Best
Lance
----------------------
JAXBRIContext.java:
- The comment “If true, despite of the specification….”; This does
not read well can this be clarified?
We have changed it to:
* If true and element namespace is not specified, namespace of parent
element will be used.
* The default value is false.
Hope this is acceptable.
Generated.java, PostConstruct.java, PreDestroy.java,Resource.java
- Be nice to use {@code} vs <code></code> for readability
- Do you really need <p></p>? I am assuming there are no
doclint errors on Generated?
- Was there a CCC for the changes as there were some minor
semantic updates
These are from JSR 250 we don't own sources.
MimeMultipart.java.:
- createMimeBodyPart @param len does not align properly
SharedInputStream.java
- writeTo @param out does not align
NameImpl.java
- Some comments have a blank line before the params, others do
not. It would be nice to be consistent
fixed
ParserPool.java
- Can you explain the status regarding returnParser() and
https://jaxp.dev.java.net/issues/show_bug.cgi?id=59
removed //TODO and commented method, Aleksej will provide patch for
resetting symbol table on parser.reset()
SAAJUtil.java
- getSystemClassLoader, do you really need to check for a
SecurityManager? Why not just AccessController.doPrivilege(could also
use a lambda also)
Removed SecurityManager check, lambda cannot be used without utilizing
multirelease-jar for saaj, our min supported version is 1.7
MethodUtil.java
- Looks like the alignment is off in the changes in invoke
There are 5 of them in the sync. I reformatted new ones. Can't see any
wrong indentation in updated ones. Also fixed package in some comments/logs.
ContextClassloaderLocal.java
- getContextClassLoader could be changed to use a lambda with
AccessController.doPrivilege if you desire
FactoryFinder.java
- getSystemProperty, it looks like you were using a lambda and
removed it for the AccessController.doPrivilege?
both no lambda for min target 1.7
SecureLoader.java
- same comment as SAAJUtil.java
I don't know the reason why SecurityManager presence check is present
there. Maybe performance? But there are 14 copies of SecureLoader class
in JAXB, if we do remove unnecessary check we should replace all of
them. Lets postpone it.
NGCCRuntimeEx.java
- copyLocater, createValidationContext @return comment needs
checked
fixed
package-info.java
- Seem the new files include a 2003/2004/1997, 2017, copyright,
should just be 2017 i believe
fixed
we are working on a new webrev, hopefully will post it later today.
Roman
On Feb 14, 2017, at 7:00 AM, Roman Grigoriadi
<roman.grigori...@oracle.com <mailto:roman.grigori...@oracle.com>> wrote:
Hi,
Please review standalone JAXB/JAXWS changes, synced to jdk/jaxws repo.
JBS: https://bugs.openjdk.java.net/browse/JDK-8174735
Webrev:
http://cr.openjdk.java.net/~aefimov/jaxws-integrations/8174735/00/
<http://cr.openjdk.java.net/%7Eaefimov/jaxws-integrations/8174735/00/>
You can find change list in the description of JBS and its linked
issues. Most diffs are for SAAJ-RI related to JDK-8166745 - remove
dependencies on jdks Xerces internal classes.
Thanks,
Roman
<http://oracle.com/us/design/oracle-email-sig-198324.gif>
<http://oracle.com/us/design/oracle-email-sig-198324.gif><http://oracle.com/us/design/oracle-email-sig-198324.gif>
<http://oracle.com/us/design/oracle-email-sig-198324.gif>Lance
Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering
1 Network Drive
Burlington, MA 01803
lance.ander...@oracle.com <mailto:lance.ander...@oracle.com>
