Hi Aleksej,
On 15/02/17 23:49, Aleks Efimov wrote:
Hi,
The new webrev with addressed comments was uploaded here:
http://cr.openjdk.java.net/~aefimov/jaxws-integrations/8174735/01
This is probably a question for the upstream project, but I'm
puzzled by this change:
http://cr.openjdk.java.net/~aefimov/jaxws-integrations/8174735/01/jaxws/src/jdk.xml.bind/share/classes/com/sun/codemodel/internal/JModuleDirective.java.frames.html
... and even more since Integer.hashCode() calls Integer.hashCode(int)
which just returns the value of the integer...
What's the purpose of calling valueOf which 1. contradict the javadoc
comment and 2. might trigger the creation of a new Integer instance?
Nit: There are also some files which present some strange
formatting/misalignment where new code/comment was added.
I have noted them below:
There's strange formatting for the anonymous subclass
of AbstractList in this file:
http://cr.openjdk.java.net/~aefimov/jaxws-integrations/8174735/01/jaxws/src/java.xml.ws/share/classes/com/sun/xml/internal/messaging/saaj/packaging/mime/internet/InternetHeaders.java.frames.html
324 headerValueView = new AbstractList<String>() {
325 @Override
326 public String get(int index) {
327 return headers.get(index).line;
328 }
329
330 @Override
331 public int size() {
332 return headers.size();
333 }
334 };
and
http://cr.openjdk.java.net/~aefimov/jaxws-integrations/8174735/01/jaxws/src/java.xml.ws/share/classes/com/sun/xml/internal/messaging/saaj/packaging/mime/internet/MimePartDataSource.java.frames.html
has also some alignment issues where @Overrides were added.
The following file has some comment alignment issues which impair
readability:
http://cr.openjdk.java.net/~aefimov/jaxws-integrations/8174735/01/jaxws/src/java.xml.ws/share/classes/com/sun/xml/internal/messaging/saaj/packaging/mime/util/ASCIIUtility.java.frames.html
best regards,
-- daniel
Best,
Aleksej
On 15/02/17 15:42, Roman Grigoriadi wrote:
Hi Mandy,
On 02/14/2017 11:53 PM, Mandy Chung wrote:
On Feb 14, 2017, at 4:00 AM, Roman Grigoriadi
<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/
jaxws/src/jdk.xml.ws/share/classes/com/sun/tools/internal/ws/wscompile/WsimportTool.java
- /** JAXWS module name. JAXWS dependency is mandatory in
generated Java module. */
- private static final String JAXWS_MODULE = "java.xml.ws";
+ /** JAXB module name. JAXB dependency is mandatory in generated
Java module. */
+ private static final String JAXWS_PACKAGE = "java.xml.ws”;
this looks to be merge failure on our side, will fix it again.
JAXWS_MODULE is the right name as we discussed in the last JAX-WS
integration to jdk9. This change should be reverted and the upstream
project should be fixed.
+ if ("-release".equals(opt) && 9 >=
getVersion(javacOptions.get(i + 1))) {
thanks, will be fixed to --release
javac supports `—-release` (double dashes, GNU long form style) but
not the single dash option. Is this new option in wsimport and wsgen
tools? It should probably be consistent with javac.
You can run jdeps —-check java.base,java.xml option to double check
if any remaining qualified exports to these modules.
Otherwise looks okay.
Mandy
