RFR: 8174735 Update JAX-WS RI integration to latest version
Roman Grigoriadi
roman.grigoriadi at oracle.com
Thu Feb 16 14:25:16 UTC 2017
Hi Daniel,
On 02/16/2017 11:39 AM, Daniel Fuchs wrote:
> 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?
Thanks for pointing out, we have removed both Integer#valueOf and
Integer#hashCode calls, it doesn't make any sense to be there. This
puzzling line resulted from useless Ingeger#hashCode(...) call which got
refactored to be target 1.7 compatible.
>
>
> 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
>
I have fixed the alignment in SAAJ.
Both changes will get to JDK next sync.
>
> best regards,
Thanks for comments,
Roman
>
> -- 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.grigoriadi at 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
>>>>
>>>
>>
>
More information about the core-libs-dev
mailing list