Re: [15]: RFR: 8244245: localizedBy() should override localized values with default values

2020-05-07 Thread Joe Wang
On 5/7/2020 9:03 AM, naoto.s...@oracle.com wrote: Hi Joe, Thank you for the review. The removed check was explicitly avoiding the default chrono/number in the locale overriding the current locale values, which is exactly what this issue is trying to remove. As Stephen wrote in another emai

Re: [15]: RFR: 8244245: localizedBy() should override localized values with default values

2020-05-07 Thread Roger Riggs
Looks good, thanks On 5/7/20 12:06 PM, naoto.s...@oracle.com wrote: Hi Roger, Thank you for the review. Wrapped the long lines as suggested, along with some typo fixes in the comments: https://cr.openjdk.java.net/~naoto/8244245/webrev.01/ Naoto On 5/7/20 7:43 AM, Roger Riggs wrote: Hi Nao

Re: [15]: RFR: 8244245: localizedBy() should override localized values with default values

2020-05-07 Thread Joe Wang
On 5/7/2020 1:14 AM, Stephen Colebourne wrote: On Thu, 7 May 2020 at 07:38, Joe Wang wrote: The Javadoc states: If the locale contains the "ca" (calendar), "nu" (numbering system), "rg" (region override), and/or "tz" (timezone) Unicode extensions, the chronology, numbering system and/o

Re: [15]: RFR: 8244245: localizedBy() should override localized values with default values

2020-05-07 Thread naoto . sato
Hi Roger, Thank you for the review. Wrapped the long lines as suggested, along with some typo fixes in the comments: https://cr.openjdk.java.net/~naoto/8244245/webrev.01/ Naoto On 5/7/20 7:43 AM, Roger Riggs wrote: Hi Naoto, Looks fine with a small source edit below. TestUnicodeExtension.

Re: [15]: RFR: 8244245: localizedBy() should override localized values with default values

2020-05-07 Thread naoto . sato
Hi Joe, Thank you for the review. The removed check was explicitly avoiding the default chrono/number in the locale overriding the current locale values, which is exactly what this issue is trying to remove. As Stephen wrote in another email, Unicode Extensions are correctly dealt in Chronolo

Re: [15]: RFR: 8244245: localizedBy() should override localized values with default values

2020-05-07 Thread Roger Riggs
Hi Naoto, Looks fine with a small source edit below. TestUnicodeExtension.java: Please wrap the excessively long lines; string concatination will put them together for the test. Thanks, Roger On 5/6/20 4:44 PM, naoto.s...@oracle.com wrote: Hello, Please review the fix to the following iss

Re: [15]: RFR: 8244245: localizedBy() should override localized values with default values

2020-05-07 Thread Stephen Colebourne
On Thu, 7 May 2020 at 07:38, Joe Wang wrote: > The Javadoc states: > If the locale contains the "ca" (calendar), "nu" (numbering > system), "rg" (region override), and/or "tz" (timezone) Unicode > extensions, the chronology, numbering system and/or the zone are overridden. > > If you remove t

Re: [15]: RFR: 8244245: localizedBy() should override localized values with default values

2020-05-06 Thread Joe Wang
Hi Naoto, The Javadoc states:     If the locale contains the "ca" (calendar), "nu" (numbering system), "rg" (region override), and/or "tz" (timezone) Unicode extensions, the chronology, numbering system and/or the zone are overridden. If you remove the two statements that check whether the sp

Re: [15]: RFR: 8244245: localizedBy() should override localized values with default values

2020-05-06 Thread Stephen Colebourne
+1 Stephen On Wed, 6 May 2020 at 21:50, wrote: > > Hello, > > Please review the fix to the following issue: > > https://bugs.openjdk.java.net/browse/JDK-8244245 > > The CSR and proposed changeset are located at: > > https://bugs.openjdk.java.net/browse/JDK-8244246 > https://cr.openjdk.java.net/~n

[15]: RFR: 8244245: localizedBy() should override localized values with default values

2020-05-06 Thread naoto . sato
Hello, Please review the fix to the following issue: https://bugs.openjdk.java.net/browse/JDK-8244245 The CSR and proposed changeset are located at: https://bugs.openjdk.java.net/browse/JDK-8244246 https://cr.openjdk.java.net/~naoto/8244245/webrev.00/ This stems from the closed issue (https: