Re: RFR 8200152: KerberosString should use UTF-8 by default

2018-04-09 Thread Weijun Wang
Thanks. Can you also review the release note at 
https://bugs.openjdk.java.net/browse/JDK-8201351?

> On Apr 8, 2018, at 11:38 AM, Xuelei Fan  wrote:
> 
> I added myself as reviewer of the CSR.
> 
> Xuelei
> 
> On 4/7/2018 8:30 PM, Weijun Wang wrote:
>> This is more natural. Thanks.
>> Updated webrev at http://cr.openjdk.java.net/~weijun/8200152/webrev.01/.
>> Do you mind reviewing the CSR also?
>> Thanks
>> Max


Re: RFR 8200152: KerberosString should use UTF-8 by default

2018-04-09 Thread Xuelei Fan
I may use a title/subject that states the new state or behavior changes 
of a RFE.  For example, "Uses UTF-8 for KerberosString".  Otherwise, 
looks fine to me.


Xuelei

On 4/9/2018 7:58 PM, Weijun Wang wrote:

Thanks. Can you also review the release note at 
https://bugs.openjdk.java.net/browse/JDK-8201351?


On Apr 8, 2018, at 11:38 AM, Xuelei Fan  wrote:

I added myself as reviewer of the CSR.

Xuelei

On 4/7/2018 8:30 PM, Weijun Wang wrote:

This is more natural. Thanks.
Updated webrev at http://cr.openjdk.java.net/~weijun/8200152/webrev.01/.
Do you mind reviewing the CSR also?
Thanks
Max


Re: RFR 8200152: KerberosString should use UTF-8 by default

2018-04-09 Thread Roger Riggs

Never mind, the latest webrev is great!

On 4/9/2018 9:03 AM, Roger Riggs wrote:

Hi,

I've seen a variety of interpretations of non-standard input for 
parsing booleans and

it is likely to cause confusion across properties.

I'd prefer that we converge on the interpretation used in the 
Boolean.parseBoolean.

Which uses:  "true".equalsIgnoreCase(s);

Better yet, call Boolean.parseBoolean!  Will be compatible with the 
previous GetBooleanAction().


$.02, Roger


On 4/7/2018 10:53 PM, Xuelei Fan wrote:

In the current implementation, the map of the property value looks like:
   null    -> false;
   ""  -> false;
   "true"  -> true;
   "false" -> false;
   "not"   -> false

For compatibility, I think you may only want to update the default 
value: null or empty property value, otherwise the behavior does not 
change.


   null    -> true;
   ""  -> true;
   "true"  -> true;
   "false" -> false;
   "not"   -> false

Per the update, "not" property means true.  It might not be an issue 
in practice.  I will have you make the final decision. Otherwise, 
looks fine to me.


Xuelei

On 3/22/2018 8:04 PM, Weijun Wang wrote:

Please take a review of the code change and CSR at

    CSR: https://bugs.openjdk.java.net/browse/JDK-8200153
    webrev: http://cr.openjdk.java.net/~weijun/8200152/webrev.00/

Thanks
Max







Re: RFR 8200152: KerberosString should use UTF-8 by default

2018-04-09 Thread Roger Riggs

Hi,

I've seen a variety of interpretations of non-standard input for parsing 
booleans and

it is likely to cause confusion across properties.

I'd prefer that we converge on the interpretation used in the 
Boolean.parseBoolean.

Which uses:  "true".equalsIgnoreCase(s);

Better yet, call Boolean.parseBoolean!  Will be compatible with the 
previous GetBooleanAction().


$.02, Roger


On 4/7/2018 10:53 PM, Xuelei Fan wrote:

In the current implementation, the map of the property value looks like:
   null    -> false;
   ""  -> false;
   "true"  -> true;
   "false" -> false;
   "not"   -> false

For compatibility, I think you may only want to update the default 
value: null or empty property value, otherwise the behavior does not 
change.


   null    -> true;
   ""  -> true;
   "true"  -> true;
   "false" -> false;
   "not"   -> false

Per the update, "not" property means true.  It might not be an issue 
in practice.  I will have you make the final decision. Otherwise, 
looks fine to me.


Xuelei

On 3/22/2018 8:04 PM, Weijun Wang wrote:

Please take a review of the code change and CSR at

    CSR: https://bugs.openjdk.java.net/browse/JDK-8200153
    webrev: http://cr.openjdk.java.net/~weijun/8200152/webrev.00/

Thanks
Max