Re: RFR 8200152: KerberosString should use UTF-8 by default
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 Fanwrote: > > 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
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 Fanwrote: 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
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
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