Re: RFR JDK-8191533,jar --describe-module prints service provider class names in lower case

2018-05-16 Thread Alan Bateman

On 16/05/2018 17:11, Xueming Shen wrote:

:

http://cr.openjdk.java.net/~sherman/8191533/webrev

Rename as suggested. The empty set change suggested seems to be an 
"incompatible"
change for the output when the set indeed is empty :-) I'm not sure if 
anyone has already
built dependency on it since 9. So if you are fine with it I'd like to 
leave it asis for now.
Aside from the modifier sets, the others can't be empty in the context 
that toString and toLowerCaseString is used. However, what you have is 
fine too.


-Alan


Re: RFR JDK-8191533,jar --describe-module prints service provider class names in lower case

2018-05-16 Thread Xueming Shen

On 5/16/18, 12:19 AM, Alan Bateman wrote:

On 16/05/2018 00:51, Xueming Shen wrote:

Hi,

Please help review the change for JDK-8191533

issue: https://bugs.openjdk.java.net/browse/JDK-8191533
webrev: http://cr.openjdk.java.net/~sherman/8191533/webrev/

The jmod change has been verify by applying jmod on those modules at 
jdk home.
This looks okay but I think it could be improved if you have 
toLowerCaseString and toString rather than toStringWithCase and 
toString. Also they can both drop the isEmpty check and prefixing the 
string with " ". Instead I think the usages should be fixed to add the 
trailing space that they are missing, e.g. " with" to " with ", " to" 
to " to " 


-Alan

Thanks Alan.

http://cr.openjdk.java.net/~sherman/8191533/webrev

Rename as suggested. The empty set change suggested seems to be an 
"incompatible"
change for the output when the set indeed is empty :-) I'm not sure if 
anyone has already
built dependency on it since 9. So if you are fine with it I'd like to 
leave it asis for now.


-Shrman


Re: RFR JDK-8191533,jar --describe-module prints service provider class names in lower case

2018-05-16 Thread Alan Bateman

On 16/05/2018 00:51, Xueming Shen wrote:

Hi,

Please help review the change for JDK-8191533

issue: https://bugs.openjdk.java.net/browse/JDK-8191533
webrev: http://cr.openjdk.java.net/~sherman/8191533/webrev/

The jmod change has been verify by applying jmod on those modules at 
jdk home.
This looks okay but I think it could be improved if you have 
toLowerCaseString and toString rather than toStringWithCase and 
toString. Also they can both drop the isEmpty check and prefixing the 
string with " ". Instead I think the usages should be fixed to add the 
trailing space that they are missing, e.g. " with" to " with ", " to" to 
" to " 


-Alan


RFR JDK-8191533,jar --describe-module prints service provider class names in lower case

2018-05-15 Thread Xueming Shen

Hi,

Please help review the change for JDK-8191533

issue: https://bugs.openjdk.java.net/browse/JDK-8191533
webrev: http://cr.openjdk.java.net/~sherman/8191533/webrev/

The jmod change has been verify by applying jmod on those modules at jdk 
home.


thanks!
sherman