> On Jun 1, 2017, at 10:29 PM, Roman Grigoriadi <roman.grigori...@oracle.com> 
> wrote:
> 
> 
>> On 1 Jun 2017, at 21:25, Mandy Chung <mandy.ch...@oracle.com 
>> <mailto:mandy.ch...@oracle.com>> wrote:
>> 
>> 
>>> On May 31, 2017, at 5:06 AM, Roman Grigoriadi <roman.grigori...@oracle.com 
>>> <mailto:roman.grigori...@oracle.com>> wrote:
>>> 
>>> Hi,
>>> 
>>> New webrev can be found here:
>>> http://cr.openjdk.java.net/~aefimov/jaxws-integrations/8176508/02/ 
>>> <http://cr.openjdk.java.net/~aefimov/jaxws-integrations/8176508/02/> 
>>> <http://cr.openjdk.java.net/~aefimov/jaxws-integrations/8176508/02/ 
>>> <http://cr.openjdk.java.net/~aefimov/jaxws-integrations/8176508/02/>>
>> 
>> jaxp/src/java.xml/share/classes/module-info.java
>>  I’m happy to see this qualified exports removed.
>> 
>> Can you update jdk/test/jdk/modules/etc/JdkQualifiedExportTest.java to
>> remove "java.xml/com.sun.xml.internal.stream.writers” from KNOWN_EXCEPTION.
>> 
>> java.xml.ws/share/classes/com/sun/xml/internal/ws/api/streaming/XMLStreamReaderFactory.java
>> java.xml.ws/share/classes/com/sun/xml/internal/ws/api/streaming/XMLStreamWriterFactory.java
>> jaxws/src/java.xml.ws/share/classes/com/sun/xml/internal/ws/util/MrJarUtil.java
>>   MrJarUtil::getNoPoolProperty is not MR specific.  Are you trying to keep
>>   the different default value when building for older release?
> 
> In standalone MrJarUtil exists and is packaged twice, but there is no need / 
> way to sync JDK8 and below version of this file. 
> Here is second non-synced version of this file for 8 and below runtime:
> https://github.com/javaee/metro-jax-ws/blob/master/jaxws-ri/rt/src/main/java/com/sun/xml/ws/util/MrJarUtil.java
>  
> <https://github.com/javaee/metro-jax-ws/blob/master/jaxws-ri/rt/src/main/java/com/sun/xml/ws/util/MrJarUtil.java>
>> 
>> It would be clearer if you want to define a constant for the default value 
>> that is subject to the runtime version.
> 
>> 
>> XMLStreamReaderFactory and XMLStreamWriterFactory would get the property 
>> value with the MR-specific default.
>> 
> 
> Than MrJarUtil would need to have only such constant for default value and 
> metho getNoPoolProperty could be moved elsewhere.



Since this is the only method right now, would it be better to check if 
Runtime.Version class is present, then use “true” as the default for the 
“noPool” property; otherwise “false”.    MrJarUtil seems a misleading name and 
maybe just move this method XMLStreamXXXFactory.  It’s a suggestion.  I won’t 
object if you want to keep this as is in JDK 9 and clean up in JDK 10 due to 
the timing.

Mandy

Reply via email to