Hi Roman, I made a pass through the webrev and have the following feedback:
src/java.xml.ws/share/classes/com/sun/xml/internal/messaging/saaj/soap/StaxLazySourceBridge.java and several files - which follow in the webrev, have formatting issues with the newly added @override and existing @overrides and should probably be cleaned up src/java.xml.ws/share/classes/com/sun/xml/internal/messaging/saaj/soap/impl/BodyElementImpl.java - can 960 -962 be deleted src/java.xml.ws/share/classes/com/sun/xml/internal/ws/util/version.properties - The copyright was reverted. Also what happens to the svn info in this file with the move to github? src/java.xml.ws/share/classes/javax/xml/soap/package-info.java - I would use ™ for TM src/java.xml.ws/share/classes/javax/xml/ws/Service.java - See comments starting at 230 seem off src/java.xml.ws/share/classes/javax/xml/ws/WebServiceRef.java - I would make the comments starting at 139 be consistent with the other comments src/jdk.xml.bind/share/classes/com/sun/tools/internal/jxc/*.properties - the copyright date was reverted src/jdk.xml.bind/share/classes/module-info.java should already be updated in the workspace src/jdk.xml.ws/share/classes/com/sun/tools/internal/ws/processor/ProcessorException.java - The copyright should be updated to 2017 src/jdk.xml.ws/share/classes/com/sun/tools/internal/ws/util/WSDLParseException.java - the copyright was reverted src/jdk.xml.ws/share/classes/com/sun/tools/internal/ws/wsdl/framework/ParseException.java - the copyright was reverted src/jdk.xml.ws/share/classes/com/sun/tools/internal/ws/wsdl/framework/ValidationException.java - the copyright was reverted src/jdk.xml.ws/share/classes/module-info.java - this was already updated in the workspace src/java.xml.bind/share/classes/javax/xml/bind/ModuleUtil.java - the copyright should only be 2017 src/java.xml.ws/share/classes/com/sun/xml/internal/ws/policy/sourcemodel/attach/ContextClassloaderLocalMessages.java. - the copyright should only be 2017 src/jdk.xml.ws/share/classes/com/sun/tools/internal/ws/resources/NewmessagesMessages.java - the copyright should only be 2017 src/jdk.xml.ws/share/classes/com/sun/tools/internal/ws/resources/newmessages.properties - this is in the workspace already > On May 3, 2017, at 12:49 PM, Roman Grigoriadi <roman.grigori...@oracle.com> > wrote: > > Hi, > > you can find new web rev here: > http://cr.openjdk.java.net/~aefimov/jaxws-integrations/8176508/01/ > <http://cr.openjdk.java.net/~aefimov/jaxws-integrations/8176508/01/> > > Previous review comments are addressed. > >> The change to version.properties reminds me to ask if there is anything in >> the jaxws repo to indicate the version of the JAX-* components? It's often >> difficult to determine what bits are in the JDK vs. the upstream project. > > > Version as in our Maven project is 2.3.0-SNAPSHOT for JAX-WS at the time we > are syncing. Subcomponents (SAAJ, JAXB mainly) are promoted, for example > in > jdk/jaxws/src/jdk.xml.bind/share/classes/com/sun/tools/internal/xjc/MessageBundle.properties > There is: > # for JDK integration - include version in source zip > jaxb.jdk.version=2.3.0-b170412.1723 > > We can add another version.properties file with versions of all JAX-* > components. We may also change version from 2.3.0-SNAPSHOT to something > unique like 2.3.0-bXXXXXX.XXXX before sync and put it to maven promoted repo. > > Roman > > >> On 12 Mar 2017, at 16:14, Alan Bateman <alan.bate...@oracle.com> wrote: >> >> >> >> On 12/03/2017 14:39, Roman Grigoriadi wrote: >>> Hi, >>> >>> Please review standalone JAXB/JAXWS changes, synced to jdk/jaxws repo. >>> >>> JBS: https://bugs.openjdk.java.net/browse/JDK-8176508 >>> Webrev: http://cr.openjdk.java.net/~aefimov/jaxws-integrations/8176508/00/ >>> >> I skimmed the changes and have a few comments (I'm sure Lance or someone >> else will do a more detailed review). >> >> In JAXBContext then "must be open to the java.xml.bind module" should be >> "must be open to at least the java.xml.bind module" so as to cover the case >> that the package is opened unconditionally or to java.xml.bind and other >> modules. In addition, include "at least" makes it consistent with other >> wording that we have agreed for other areas. >> >> In MailcapCommandMap then the following doesn't seem right for the class >> description: >> >> 59 * (Where <i>java.home</i> is the value of the "java.home" System property >> 60 * and <i>conf</i> is the directory named "conf" if it exists, >> 61 * otherwise the directory named "lib"; the "conf" directory was >> 62 * introduced in JDK 1.9.) >> >> It might be simpler to just have javadoc specify that it attepts to locate >> the `mailcap` file in the Java run-time image and then add an @implNote with >> the details as to where it looks for specific runtime releases. >> >> I see the new source file ModuleUtil is using java.util.StringTokenizer. >> It's use in new code has been discouraged for many years and maybe this >> could start out using String.split rather than the legacy class. >> >> The change to version.properties reminds me to ask if there is anything in >> the jaxws repo to indicate the version of the JAX-* components? It's often >> difficult to determine what bits are in the JDK vs. the upstream project. >> >> -Alan > <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>
