Re: RFR: 8280400: JDK 19 L10n resource files update - msgdrop 10

2022-03-09 Thread Joe Wang
On Wed, 9 Mar 2022 21:09:30 GMT, Alisen Chung wrote: > msg drop for jdk19, Mar 9, 2022 For the bundles in java.xml: For files with Oracle copyright, update the year to 2022 and @LastModified Mar 2022. Take XPATHErrorResources_ja.java as an example, the copyright year was updated to 2021 and @

Re: RFR: 8282929: Localized monetary symbols are not reflected in `toLocalizedPattern` return value [v2]

2022-03-11 Thread Joe Wang
On Fri, 11 Mar 2022 22:20:38 GMT, Naoto Sato wrote: >> `DecimalFormat.toLocalizedPattern()` was not honoring the monetary >> decimal/grouping separator symbols. Fix is straightforward to use the >> correct symbols depending on the formatter type. > > Naoto Sato has updated the pull request incr

Re: RFR: 8283277: ISO 4217 Amendment 171 Update

2022-03-17 Thread Joe Wang
On Thu, 17 Mar 2022 18:10:17 GMT, Naoto Sato wrote: > This is to incorporate the ISO 4217 amendment 171 for Sierra Leonean LEONE > redenomination (removing 3 zeros). Its effective date is 4/1, but I went > ahead as JDK19 won't be released by 4/1. Marked as reviewed by joehw (Reviewer). --

Re: RFR: 8283698: Refactor Locale constructors used in src/test

2022-04-06 Thread Joe Wang
On Wed, 6 Apr 2022 17:45:13 GMT, Naoto Sato wrote: > This is a follow-on task after deprecating the Locale constructors > (https://bugs.openjdk.java.net/browse/JDK-8282819). Most of the changes are > simple replacements to Locale constructors with `Locale.of()` or Locale > constants, such as `

Re: RFR: 8283698: Refactor Locale constructors used in src/test

2022-04-06 Thread Joe Wang
On Wed, 6 Apr 2022 17:45:13 GMT, Naoto Sato wrote: > This is a follow-on task after deprecating the Locale constructors > (https://bugs.openjdk.java.net/browse/JDK-8282819). Most of the changes are > simple replacements to Locale constructors with `Locale.of()` or Locale > constants, such as `

Re: RFR: 8283698: Refactor Locale constructors used in src/test

2022-04-06 Thread Joe Wang
On Wed, 6 Apr 2022 17:45:13 GMT, Naoto Sato wrote: > This is a follow-on task after deprecating the Locale constructors > (https://bugs.openjdk.java.net/browse/JDK-8282819). Most of the changes are > simple replacements to Locale constructors with `Locale.of()` or Locale > constants, such as `

Re: RFR: 8283698: Refactor Locale constructors used in src/test

2022-04-06 Thread Joe Wang
On Thu, 7 Apr 2022 01:16:27 GMT, Naoto Sato wrote: >> test/jdk/java/text/Format/DateFormat/DateFormatRoundTripTest.java line 81: >> >>> 79: >>> 80: /** >>> 81: * Parse a name like "fr_FR" into Locale.of("fr", "FR", ""); >> >> Locale.France? > > The test code parses the input string (e

Re: RFR: 8283698: Refactor Locale constructors used in src/test

2022-04-06 Thread Joe Wang
On Wed, 6 Apr 2022 17:45:13 GMT, Naoto Sato wrote: > This is a follow-on task after deprecating the Locale constructors > (https://bugs.openjdk.java.net/browse/JDK-8282819). Most of the changes are > simple replacements to Locale constructors with `Locale.of()` or Locale > constants, such as `

Re: RFR: 8283698: Refactor Locale constructors used in src/test

2022-04-06 Thread Joe Wang
On Thu, 7 Apr 2022 01:16:32 GMT, Naoto Sato wrote: >> test/jdk/java/text/Format/NumberFormat/CurrencyFormat.java line 63: >> >>> 61: Locale.of("it", "IT", "EURO"), >>> 62: Locale.forLanguageTag("de-AT"), >>> 63: Locale.forLanguageTag("fr-CH"), >> >> Use the n

Re: RFR: 8265315: Support for CLDR version 41

2022-04-07 Thread Joe Wang
On Thu, 7 Apr 2022 21:20:20 GMT, Naoto Sato wrote: > This is to upgrade the CLDR data from version 39 to version 41 which was > released yesterday. The vast majority of the changes are basically replacing > the CLDR data, along with tools/testcase alignments. Here is the link to CLDR > v41's r

Re: RFR: 8265315: Support for CLDR version 41 [v2]

2022-04-08 Thread Joe Wang
On Fri, 8 Apr 2022 20:17:52 GMT, Naoto Sato wrote: >> This is to upgrade the CLDR data from version 39 to version 41 which was >> released yesterday. The vast majority of the changes are basically replacing >> the CLDR data, along with tools/testcase alignments. Here is the link to >> CLDR v41

Re: RFR: 8279185: Support for IsoFields in JapaneseDate/MinguoDate/ThaiBuddhistDate [v5]

2022-04-12 Thread Joe Wang
On Tue, 12 Apr 2022 20:33:53 GMT, Naoto Sato wrote: >> Supporting `IsoFields` temporal fields in chronologies that are similar to >> ISO chronology. Corresponding CSR has also been drafted. > > Naoto Sato has updated the pull request with a new target base due to a merge > or a rebase. The incr

Re: RFR: 8279185: Support for IsoFields in JapaneseDate/MinguoDate/ThaiBuddhistDate [v5]

2022-04-12 Thread Joe Wang
On Tue, 12 Apr 2022 20:33:53 GMT, Naoto Sato wrote: >> Supporting `IsoFields` temporal fields in chronologies that are similar to >> ISO chronology. Corresponding CSR has also been drafted. > > Naoto Sato has updated the pull request with a new target base due to a merge > or a rebase. The incr

Re: RFR: 8186958: Need method to create pre-sized HashMap [v16]

2022-04-13 Thread Joe Wang
On Wed, 13 Apr 2022 16:29:11 GMT, XenoAmess wrote: >> 8186958: Need method to create pre-sized HashMap > > XenoAmess has updated the pull request incrementally with one additional > commit since the last revision: > > revert changes in: > src/java.desktop > src/java.management > src/jdk

Re: RFR: 8186958: Need method to create pre-sized HashMap [v18]

2022-04-13 Thread Joe Wang
On Thu, 14 Apr 2022 01:15:05 GMT, XenoAmess wrote: >> src/java.xml/share/classes/com/sun/org/apache/xalan/internal/xsltc/dom/DocumentCache.java >> line 171: >> >>> 169: _current = 0; >>> 170: _size = size; >>> 171: _references = HashMap.newHashMap(_size); >> >> Not `_s

Re: RFR: 8186958: Need method to create pre-sized HashMap [v18]

2022-04-13 Thread Joe Wang
On Thu, 14 Apr 2022 01:13:18 GMT, XenoAmess wrote: >> src/java.xml/share/classes/com/sun/org/apache/xerces/internal/impl/xs/traversers/XSAttributeChecker.java >> line 1819: >> >>> 1817: Map items; >>> 1818: LargeContainer(int size) { >>> 1819: items = HashMap.newHashMap(size*2+1

Re: RFR: 8186958: Need method to create pre-sized HashMap [v20]

2022-04-14 Thread Joe Wang
On Thu, 14 Apr 2022 17:05:39 GMT, XenoAmess wrote: >> 8186958: Need method to create pre-sized HashMap > > XenoAmess has updated the pull request incrementally with one additional > commit since the last revision: > > revert changes on ProcessEnvironment src/java.xml/share/classes/com/sun/or

Re: RFR: 8186958: Need method to create pre-sized HashMap [v20]

2022-04-14 Thread Joe Wang
On Thu, 14 Apr 2022 18:05:48 GMT, XenoAmess wrote: >> src/java.xml/share/classes/com/sun/org/apache/xalan/internal/xsltc/dom/DocumentCache.java >> line 3: >> >>> 1: /* >>> 2: * Copyright (c) 2015, 2022, Oracle and/or its affiliates. All rights >>> reserved. >>> 3: */ >> >> The LastModified

Re: RFR: 8186958: Need method to create pre-sized HashMap [v21]

2022-04-14 Thread Joe Wang
On Thu, 14 Apr 2022 18:10:28 GMT, XenoAmess wrote: >> 8186958: Need method to create pre-sized HashMap > > XenoAmess has updated the pull request incrementally with one additional > commit since the last revision: > > add `@LastModified: Apr 2022` to DocumentCache Marked as reviewed by joehw

Re: RFR: 8286154: Fix 3rd party notices in test files

2022-05-05 Thread Joe Wang
On Thu, 5 May 2022 16:13:59 GMT, Naoto Sato wrote: > Trivial fix to 3rd party copyright notices. Marked as reviewed by joehw (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8558

Re: RFR: 8285844: TimeZone.getTimeZone(ZoneOffset) does not work for all ZoneOffsets and returns GMT unexpected [v4]

2022-05-11 Thread Joe Wang
On Wed, 11 May 2022 17:04:41 GMT, Naoto Sato wrote: >> This is to extend the `Custom ID`s in `java.util.TimeZone` class to support >> second-level resolution, enabling round trips with `java.time.ZoneOffset`s. >> Corresponding CSR is also being drafted. > > Naoto Sato has updated the pull reque

Re: RFR: 8285844: TimeZone.getTimeZone(ZoneOffset) does not work for all ZoneOffsets and returns GMT unexpected [v5]

2022-05-11 Thread Joe Wang
On Wed, 11 May 2022 20:04:39 GMT, Naoto Sato wrote: >> This is to extend the `Custom ID`s in `java.util.TimeZone` class to support >> second-level resolution, enabling round trips with `java.time.ZoneOffset`s. >> Corresponding CSR is also being drafted. > > Naoto Sato has updated the pull reque

Re: RFR: 8279185: Support for IsoFields in JapaneseDate/MinguoDate/ThaiBuddhistDate [v10]

2022-05-20 Thread Joe Wang
On Tue, 17 May 2022 23:40:04 GMT, Naoto Sato wrote: >> Supporting `IsoFields` temporal fields in chronologies that are similar to >> ISO chronology. Corresponding CSR has also been drafted. > > Naoto Sato has updated the pull request with a new target base due to a merge > or a rebase. The incr

Re: RFR: 8287187: Utilize HashMap.newHashMap() in CLDRConverter [v2]

2022-05-25 Thread Joe Wang
On Wed, 25 May 2022 17:15:18 GMT, Naoto Sato wrote: >> Refactoring the leftover self-calculations of the optimized `HashMap` >> initial value with `newHashMap()` method. Also replaced some string literals >> using text blocks for better readability. Confirmed that the output resource >> bundle

Re: RFR: 8287340: Refactor old code using StringTokenizer in locale related code

2022-05-31 Thread Joe Wang
On Tue, 31 May 2022 17:46:18 GMT, Naoto Sato wrote: > Refactoring some old code in locale providers. The test case data have also > been modified due to: > - There's a bug in `LocaleProviderAdapter.toLocaleArray()` where it did not > handle the case for `no-NO-NY`. > - `Locale.toLanguageTag()`

Re: [14] RFR: 8233579: DateFormatSymbols.getShortMonths() return wrong string on es_CL, es_CO locales

2019-11-06 Thread Joe Wang
Looks good to me. It might be good to leave a note to the method about the change (e.g. why it no longer substitutes). -Joe On 11/6/19 1:45 PM, naoto.s...@oracle.com wrote: Hi, Please review the fix to the following issue: https://bugs.openjdk.java.net/browse/JDK-8233579 The proposed change

Re: [14] RFR: 8233579: DateFormatSymbols.getShortMonths() return wrong string on es_CL, es_CO locales

2019-11-06 Thread Joe Wang
modified the JIRA issue mentioning it, so that maintainers later could find out the rationale. Naoto On 11/6/19 6:21 PM, Joe Wang wrote: Looks good to me. It might be good to leave a note to the method about the change (e.g. why it no longer substitutes). -Joe On 11/6/19 1:45 PM, naoto.s

Re: [14] RFR: 8222756: Plural support in CompactNumberFormat

2019-12-04 Thread Joe Wang
Hi Naoto, Looks good. I understand you'll update the webrev (with the added statement to readObject) once the CSR is approved. ResourceBundleGenerator.java might have been accidentally touched as there's no change there. I wonder if you need to guard the pluralRules input since you're buil

Re: [14] RFR: 8222756: Plural support in CompactNumberFormat

2019-12-05 Thread Joe Wang
+1, looks good! Best regards, Joe On 12/5/19 12:13 PM, Roger Riggs wrote: Hi Naoto, Thanks for the updates. Looks fine. Roger On 12/5/19 1:49 PM, naoto.s...@oracle.com wrote: Thanks, Joe and Roger, for the reviews. Here is the updated webrev: http://cr.openjdk.java.net/~naoto/8222756/w

Re: RFR 8235238: Parsing a time string ignores any custom TimeZoneNameProvider

2019-12-12 Thread Joe Wang
Hi Naoto, Does the new code block, 4156 - 4174, need a conditional statement, that is when it's for a standard timezon? Before this change, or before a custom TimeZoneNameProvider is attempted, the process didn't need to loop through regionIds to add display names. Thanks, Joe On 12/11/19 1

Re: RFR 8235238: Parsing a time string ignores any custom TimeZoneNameProvider

2019-12-12 Thread Joe Wang
. The changes look good to me then. Best, Joe Naoto On 12/12/19 11:32 AM, Joe Wang wrote: Hi Naoto, Does the new code block, 4156 - 4174, need a conditional statement, that is when it's for a standard timezon? Before this change, or before a custom TimeZoneNameProvider is attempted, the proc

Re: RFR 8235238: Parsing a time string ignores any custom TimeZoneNameProvider

2019-12-12 Thread Joe Wang
, Roger Riggs wrote: +1 There's quite a bit of work being done to get the eligible stings as part of each parse but it doesn't look easy to re-use it acrosses parses. Roger On 12/12/19 3:42 PM, Joe Wang wrote: On 12/12/19 12:31 PM, naoto.s...@oracle.com wrote: Hi Joe, Thank y

Re: [15] RFR: 8227313: Support monetary grouping separator in DecimalFormat/DecimalFormatSymbols

2019-12-23 Thread Joe Wang
Hi Naoto, Is there a need for an APINote to note the relationship between the new get/setMonetaryGroupingSeparator and get/setGroupingSeparator methods. The new method did state it "May be different from {@code grouping separator} in some locales", but that may be insufficient. For example, d

Re: [15] RFR: 8227313: Support monetary grouping separator in DecimalFormat/DecimalFormatSymbols

2020-01-02 Thread Joe Wang
Happy New Year, Naoto! Thanks for the explanation and changes. The changeset looks good to me. -Joe On 1/2/20 12:50 PM, naoto.s...@oracle.com wrote: Hi Joe, Happy new year and thanks for your comments. Please see my replies below: On 12/23/19 5:20 PM, Joe Wang wrote: Hi Naoto, Is there a

Re: [15] RFR: 8227313: Support monetary grouping separator in DecimalFormat/DecimalFormatSymbols

2020-01-03 Thread Joe Wang
changeset, as the cached hash code in DecimalFormatSymbols needs to be recalculated when any of the relevant fields is mutated. Here is the updated webrev: http://cr.openjdk.java.net/~naoto/8227313/webrev.02/ Naoto On 1/2/20 2:19 PM, Joe Wang wrote: Happy New Year, Naoto! Thanks for th

Re: [15] RFR: 8227313: Support monetary grouping separator in DecimalFormat/DecimalFormatSymbols

2020-01-03 Thread Joe Wang
x27;s why he/she did not bother make them in sync, IMO. So there seems to be no explicit reason (to be noted in the source code) for not syncing. My $.02 Naoto On 1/3/20 11:40 AM, Joe Wang wrote: Hi Naoto, The change looks fine to me as only monetaryGroupingSeparator was added to

Re: [15] RFR: 8174270: Consolidate ICU sources in one location

2020-01-10 Thread Joe Wang
Is there a reason why uidna.spp was left out of the move? -Joe On 1/10/20 2:02 PM, naoto.s...@oracle.com wrote: Hi, Please review the fix to the following issue: https://bugs.openjdk.java.net/browse/JDK-8174270 The proposed changeset is located at: https://cr.openjdk.java.net/~naoto/8174270

Re: [15] RFR: 8174270: Consolidate ICU sources in one location

2020-01-10 Thread Joe Wang
I see. It's all good to me then. Best, Joe On 1/10/20 4:04 PM, naoto.s...@oracle.com wrote: Hi Joe, That data file seems no longer included in the ICU4J package (as of 64.2), thus I left it as it is. Naoto On 1/10/20 2:57 PM, Joe Wang wrote: Is there a reason why uidna.spp was lef

Re: [15] RFR: 8187987: Add a mechanism to configure custom variants in HijrahChronology

2020-01-14 Thread Joe Wang
Hi Naoto, Since it's dealing with non-standard properties files, is there a need to verify the files? The constructor (HijrahChronology) does check whether the id or type is empty. If there is no existing process to validate, it's probably not worth it to spend time as it's rare and it's conf

Re: [15] RFR: 8187987: Add a mechanism to configure custom variants in HijrahChronology

2020-01-14 Thread Joe Wang
On 1/14/20 6:04 PM, naoto.s...@oracle.com wrote: Hi Joe, Thank you for the review. Please find my comments below: On 1/14/20 3:35 PM, Joe Wang wrote: Hi Naoto, Since it's dealing with non-standard properties files, is there a need to verify the files? The constructor (HijrahChron

Re: [14] RFR (XXS) 8238605: Correct the CLDR version number in cldr.md files

2020-02-06 Thread Joe Wang
+1 Best, Joe On 2/6/20 8:50 AM, naoto.s...@oracle.com wrote: Hello, Please review this extra small fix for this issue: https://bugs.openjdk.java.net/browse/JDK-8238605 The proposed changeset is located at: http://cr.openjdk.java.net/~naoto/8238605/webrev.00/ It is simply updating the vers

Re: [15] RFR: 8234347: "Turkey" meta time zone does not generate composed localized names, 8236548: Localized time zone name inconsistency between English and other locales

2020-02-07 Thread Joe Wang
Hi Naoto, I see the existing tests were changed, e.g. the abbreviation / short timezone name, the result of calling getDisplayName. Would you need a CSR to discuss/document the potential incompatibility? Best, Joe On 2/7/20 1:44 PM, naoto.s...@oracle.com wrote: Hello, Please review the fix

Re: [15] RFR: 8234347: "Turkey" meta time zone does not generate composed localized names, 8236548: Localized time zone name inconsistency between English and other locales

2020-02-11 Thread Joe Wang
+1. That's nicer. -Joe On 2/11/20 10:17 AM, naoto.s...@oracle.com wrote: Hi, I modified the proposed changeset. Removed the hard coded 3-letter id compatibility mappings (oldMappings) from CLDRConverter.java. Instead, using public ZoneId.SHORT_IDS that contain the same set of mappings (wasn

Re: [15] RFR: 8240626: Some of the java.time.chrono.Eras return empty display name for some styles and locales

2020-03-13 Thread Joe Wang
Hi Naoto, The existing tests verifies that a display name matches an expected value. I wonder if you'd want to do a bit more than the Boolean assertion with a similar approach as the existing test, that is, check that the fallback values/alias names match expected names. Best, Joe On 3/13/

Re: [15] RFR: 8240626: Some of the java.time.chrono.Eras return empty display name for some styles and locales

2020-03-13 Thread Joe Wang
parents, unless parsing CLDR's source XML files in the test at the runtime. I think it is enough to just ensure there's no empty names returned at the runtime, IMO. Naoto On 3/13/20 5:00 PM, Joe Wang wrote: Hi Naoto, The existing tests verifies that a display name matches an expected

Re: [15] RFR: 8241082: Upgrade IANA Language Subtag Registry data to 03-16-2020 version

2020-03-17 Thread Joe Wang
Hi Naoto, Looks good to me. -Joe On 3/17/20 1:58 PM, naoto.s...@oracle.com wrote: Hello, Please review the fix to the following issue: https://bugs.openjdk.java.net/browse/JDK-8241082 The proposed changeset is located at: http://cr.openjdk.java.net/~naoto/8241082/webrev.00/ It is simply u

Re: [15] RFR: 8243664: JavaDoc of CompactNumberFormat points to wrong enum

2020-04-27 Thread Joe Wang
+1. Indeed, the resulting javadoc was fine :-) Best, Joe On 4/27/2020 9:36 AM, naoto.s...@oracle.com wrote: Hello, Please review this small doc fix to the following issue: https://bugs.openjdk.java.net/browse/JDK-8243664 Here is the diff: --- --- old/src/java.base/share/classes/java/tex

Re: [15] RFR: 8244152: Remove unnecessary hash map resize in LocaleProviderAdapters

2020-04-29 Thread Joe Wang
+1 -Joe On 4/29/2020 3:18 PM, naoto.s...@oracle.com wrote: Hello, Please review this small fix to the following issue: https://bugs.openjdk.java.net/browse/JDK-8244152 The proposed changeset is located at: https://cr.openjdk.java.net/~naoto/8244152/webrev.00/ The hash map used there didn't

Re: [15] RFR: 8244459: Optimize the hash map size in LocaleProviderAdapters

2020-05-05 Thread Joe Wang
Hi Naoto, It seems you've missed the parentheses in : Set tagset = new HashSet<>(tokens.countTokens() * 4 + 2 / 3); vs Set tagset = new HashSet<>((tokens.countTokens() * 4 + 2) / 3); -Joe On 5/5/2020 11:01 AM, naoto.s...@oracle.com wrote: And here is the fix. Please review. http://cr.openjdk.

Re: [15] RFR: 8244459: Optimize the hash map size in LocaleProviderAdapters

2020-05-05 Thread Joe Wang
On 5/5/2020 2:08 PM, naoto.s...@oracle.com wrote: Thanks, all. I didn't see this coming! +1, just when one might think it was just a minor tweak... ;-) If I understand the discussion correctly, Peter's suggestion is the most optimal (Mark, your formula produces 1 for the expected size is

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-07 Thread Joe Wang
email, Unicode Extensions are correctly dealt in Chronology.ofLocale()/DecimalStyle.of() methods indirectly, so I believe no doc change is warranted. Ok, thanks for the explanation. -Joe Naoto On 5/6/20 11:32 PM, Joe Wang wrote: Hi Naoto, The Javadoc states: If the locale contains the

Re: [15] RFR: 8245241: Incorrect locale provider preference is not logged

2020-05-20 Thread Joe Wang
Hi Naoto, I don't seem to see the DateFormat class or the getDateInstance methods specify how errors may be handled (or logged). Is that stated somewhere else?  In other cases, I see that you've changed it to throw ServiceConfigurationError, that looks to me may be better than a log as a conf

Re: [15] RFR: 8245241: Incorrect locale provider preference is not logged

2020-05-20 Thread Joe Wang
Hi Naoto, Thanks for the explanation. I agree on the needs for maintaining backward compatibility.  The patch looks good me then. Best, Joe On 5/20/2020 4:49 PM, naoto.s...@oracle.com wrote: Hi Joe, thanks for the review. On 5/20/20 4:10 PM, Joe Wang wrote: Hi Naoto, I don't seem t

Re: [15] RFR: 8239480: Support for CLDR version 37

2020-05-20 Thread Joe Wang
Looks good to me. A minor comment: in the license files, Terms of Use lost its original format (e.g. headings and numbering), lines are also super long. But I think we've seen them in the previous update, it's up to you whether you'd want to reformat th

Re: [15] RFR: 8239480: Support for CLDR version 37

2020-05-20 Thread Joe Wang
the lines stretched very long <https://cr.openjdk.java.net/~naoto/8239480/webrev.00/src/jdk.localedata/share/legal/cldr.md.html>. But again, the format of the md files might not be that important. Your call. -Joe Naoto On 5/20/20 6:51 PM, Joe Wang wrote: Looks good to me. A minor com

Re: RFR: 8248695: HostLocaleProviderAdapterImpl provides invalid date-only

2020-07-13 Thread Joe Wang
Hi Naoto, Would it make sense to provide an additional test using the public APIs similar to the one provided in the bug report? I'm sure yours is correct and covers more cases than the original, but it would be nice to have an actual use case and use the public APIs. The report showed it was

Re: RFR: 8248695: HostLocaleProviderAdapterImpl provides invalid date-only

2020-07-13 Thread Joe Wang
On 7/13/2020 7:01 PM, naoto.s...@oracle.com wrote: Hi Joe, Thank you for your review. On 7/13/20 3:55 PM, Joe Wang wrote: Hi Naoto, Would it make sense to provide an additional test using the public APIs similar to the one provided in the bug report? I'm sure yours is correct and c

Re: RFR: 8248695: HostLocaleProviderAdapterImpl provides invalid date-only

2020-07-13 Thread Joe Wang
On 7/13/2020 9:04 PM, naoto.s...@oracle.com wrote: Hi Joe, On 7/13/20 7:28 PM, Joe Wang wrote: On 7/13/2020 7:01 PM, naoto.s...@oracle.com wrote: Hi Joe, Thank you for your review. On 7/13/20 3:55 PM, Joe Wang wrote: Hi Naoto, Would it make sense to provide an additional test using

Re: RFR: 8248655: Support supplementary characters in String case insensitive operations

2020-07-15 Thread Joe Wang
Hi Naoto, In StringUTF16.java, if one is isHighSurrogate and the other not, you may quickly return without going through the rest of the process, probably not significant as cp1 and cp2 and/or u1 and u2 won't be equal anyways. But it could skip a couple of toCodePoint/toUpperCase/toLowerCase

Re: RFR: 8248655: Support supplementary characters in String case insensitive operations

2020-07-15 Thread Joe Wang
2020, at 3:32 PM, naoto.s...@oracle.com wrote: Hi Joe, Thank you for your review. On 7/15/20 10:57 AM, Joe Wang wrote: Hi Naoto, In StringUTF16.java, if one is isHighSurrogate and the other not, you may quickly return without going through the rest of the process, probably not significant as c

Re: RFR: 8248655: Support supplementary characters in String case insensitive operations

2020-07-20 Thread Joe Wang
Hi Naoto, StringUTF16: line 384 - 388 seem unnecessary since you'd only get there if 389:isHighSurrogate is not true. But more importantly, StringUTF16 has existing method "codePointAt" you may want to consider instead of adding a new method. Comparing to the base benchmark: StringCompareToI

Re: RFR: 8248655: Support supplementary characters in String case insensitive operations

2020-07-20 Thread Joe Wang
ava.net/~naoto/8248655.8248434/webrev.04/ Naoto On 7/20/20 2:39 PM, naoto.s...@oracle.com wrote: Hi Joe, Thank you for your comments. On 7/20/20 11:20 AM, Joe Wang wrote: Hi Naoto, StringUTF16: line 384 - 388 seem unnecessary since you'd only get there if 389:isHighSurrogate is n

Re: RFR: 8248655: Support supplementary characters in String case insensitive operations

2020-07-21 Thread Joe Wang
On 7/20/2020 8:58 PM, naoto.s...@oracle.com wrote: The short-cut worked well. There's maybe a further optimization we could do to rid us of the performance concern (or the need to run additional performance tests). Consider the cases where strings in comparison don't contain any sup characte

Re: RFR: 8248655: Support supplementary characters in String case insensitive operations

2020-07-22 Thread Joe Wang
Hi Naoto, The change looks good to me. "supLower" is indeed super slow :-) The only minor comment I have is that the compareCodePointCI method performs toUpperCase unconditionally. That's not a problem for the regular case, where a check on cp1 == cp2 (line 337) is done prior to the method ca

Re: RFR: 8251499: no-placeholder compact number patterns throw IllegalArgumentException

2020-08-14 Thread Joe Wang
Hi Naoto, Looks good to me. While a negative divisor representing no zeros is newly introduced, the "divisor > 0" checks seem to have always been beneficial.  I had to count the number of ""s in COMPACT_PATTERN13 :-) Have a great weekend! Joe On 8/14/2020 3:20 PM, naoto.s...@oracle.com wrot

Re: RFR: 8251499: no-placeholder compact number patterns throw IllegalArgumentException

2020-08-17 Thread Joe Wang
: http://cr.openjdk.java.net/~naoto/8251499/webrev.01/ I added a new test case (COMPACT_PATTERN14), which actually is extracted from CLDR 38 Somali locale that demonstrates the issue. I'd appreciate your further review. Naoto On 8/14/20 6:21 PM, Joe Wang wrote: Hi Naoto, Looks good

Re: RFR: 8251499: no-placeholder compact number patterns throw IllegalArgumentException

2020-08-18 Thread Joe Wang
sults from the method (cnfMultiplier and whether to return immediately for no-placeholder cases). https://cr.openjdk.java.net/~naoto/8251499/webrev.02/ Naoto On 8/17/20 11:52 PM, Joe Wang wrote: Hi Naoto, Looks good overall. One nit, blocks 1633-1639 and 1642-1649 may share a common private m

Re: RFR: 8252552: DecimalFormat javadoc contains HTML tags in example code

2020-08-31 Thread Joe Wang
+1 Or, maybe is not needed for code comments -Joe On 8/31/20 10:14 AM, Lance Andersen wrote: Hi Naoto, Looks OK to me. Best Lance On Aug 31, 2020, at 12:22 PM, Naoto Sato wrote: Hello, Please review this simple javadoc fix to: https://bugs.openjdk.java.net/browse/JDK-8252552 The prop

Re: RFR: 8220483: Calendar.setTime(Date date) throws NPE with Date date = null

2020-09-14 Thread Joe Wang
On Mon, 14 Sep 2020 22:41:55 GMT, Lance Andersen wrote: >> Hi, >> >> Please review this simple doc fix. > > Hi Naoto, > > The change looks fine. I might have created a CSR just to track the doc > change. The change looks fine. However, are other methods in the same situation, e.g. void setT

Re: RFR: 8220483: Calendar.setTime(Date date) throws NPE with Date date = null

2020-09-14 Thread Joe Wang
On Mon, 14 Sep 2020 23:15:10 GMT, Naoto Sato wrote: >> Thanks, Lance, >> >>> The change looks fine. I might have created a CSR just to track the doc >>> change. >> >> Yes, and thanks for the CSR review too! > > Thanks, Joe. > >> The change looks fine. However, are other methods in the same si

Re: RFR: 8220483: Calendar.setTime(Date date) throws NPE with Date date = null

2020-09-14 Thread Joe Wang
On Mon, 14 Sep 2020 22:18:34 GMT, Naoto Sato wrote: > Hi, > > Please review this simple doc fix. Marked as reviewed by joehw (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/159

Re: RFR: 8253153: Mentioning of "hour-of-minute" in java.time.temporal.TemporalField JavaDoc

2020-09-17 Thread Joe Wang
On Fri, 18 Sep 2020 01:49:09 GMT, Naoto Sato wrote: > Hi, > > Please review this simple doc fix. Marked as reviewed by joehw (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/234

Re: RFR: 8253321: java.util.Locale.LanguageRange#equals is inconsistent after calling hashCode

2020-09-18 Thread Joe Wang
On Fri, 18 Sep 2020 23:26:39 GMT, Naoto Sato wrote: > Hi, > > Please review the fix to JDK-8253321. As in the issue, uninitialized (cached) > hash code was incorrectly referenced in > equals() method. Removing it will correct the problem. Also, unrelated to the > issue, I fixed a parameter des

Re: RFR: 8253321: java.util.Locale.LanguageRange#equals is inconsistent after calling hashCode

2020-09-18 Thread Joe Wang
On Fri, 18 Sep 2020 23:26:39 GMT, Naoto Sato wrote: > Hi, > > Please review the fix to JDK-8253321. As in the issue, uninitialized (cached) > hash code was incorrectly referenced in > equals() method. Removing it will correct the problem. Also, unrelated to the > issue, I fixed a parameter des

Re: RFR: 8253321: java.util.Locale.LanguageRange#equals is inconsistent after calling hashCode [v2]

2020-09-18 Thread Joe Wang
On Sat, 19 Sep 2020 01:36:38 GMT, Naoto Sato wrote: >> Hi, >> >> Please review the fix to JDK-8253321. As in the issue, uninitialized >> (cached) hash code was incorrectly referenced in >> equals() method. Removing it will correct the problem. Also, unrelated to >> the issue, I fixed a paramet

Re: RFR: 8255086: Update the root locale display names [v2]

2020-10-22 Thread Joe Wang
On Thu, 22 Oct 2020 21:40:59 GMT, Brent Christian wrote: >> Naoto Sato has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Put a newline at the end. > > Marked as reviewed by bchristi (Reviewer). Hi Naoto, Do we need a test similar to ISO3

Re: RFR: 8255086: Update the root locale display names [v2]

2020-10-22 Thread Joe Wang
On Thu, 22 Oct 2020 23:20:08 GMT, Naoto Sato wrote: > Hi Joe, > > > Do we need a test similar to ISO3166 where display names of Locale.ROOT and > > Locale.US are compared? I see LocaleEnhanceTest.java has references to > > Locale.ROOT and a few selected names were updated. But we don't seem to

Re: RFR: 8255086: Update the root locale display names [v2]

2020-10-22 Thread Joe Wang
On Wed, 21 Oct 2020 23:38:23 GMT, Naoto Sato wrote: >> Hi, >> >> Please review the changes to the subject issue. The fix replaces the >> obsolete/incorrect English language/region/script display names in the >> COMPAT root locale bundle. The data are derived from CLDR's English names. > > Naot

Re: RFR: 8255671: Bidi.reorderVisually has misleading exception messages

2020-10-30 Thread Joe Wang
On Fri, 30 Oct 2020 22:23:30 GMT, Naoto Sato wrote: > Hi, > > Please review this simple message fix that follows JDK-8255242. > > Naoto Marked as reviewed by joehw (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/973

Re: RFR: 8247781: Day periods support [v5]

2020-11-04 Thread Joe Wang
On Fri, 30 Oct 2020 11:42:49 GMT, Stephen Colebourne wrote: >> Naoto Sato has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Fixed TCK test failures, added the new pattern char description in >> DateTimeFormatter > > Changes requested by

Re: RFR: 8247781: Day periods support [v7]

2020-11-05 Thread Joe Wang
On Thu, 5 Nov 2020 17:12:11 GMT, Naoto Sato wrote: >> Hi, >> >> Please review the changes for the subject issue. This is to enhance the >> java.time package to support day periods, such as "in the morning", defined >> in CLDR. It will add a new pattern character 'B' and its supporting builder

Re: RFR: 8251317: Support for CLDR version 38

2020-11-18 Thread Joe Wang
On Tue, 17 Nov 2020 23:19:23 GMT, Naoto Sato wrote: > Hi, > > Please review the changes for upgrading the CLDR data to version 38. The vast > majority of the changes are simply the changes in CLDR upstream, and others > are mainly test changes due to the locale data change. Looks good to me.

Re: RFR: 8247432: Update IANA Language Subtag Registry to Version 2020-09-29

2020-11-20 Thread Joe Wang
On Fri, 20 Nov 2020 17:55:55 GMT, Naoto Sato wrote: > Hi, > > Please review the changes to the subject issue. This is to incorporate the > latest language subtag registry definition into the JDK. Marked as reviewed by joehw (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/

Re: RFR: 8257964: Broken Calendar#getMinimalDaysInFirstWeek with java.locale.providers=HOST

2020-12-10 Thread Joe Wang
On Thu, 10 Dec 2020 21:12:29 GMT, Naoto Sato wrote: > Hello, > > Please review the changes to the subject issue. getMinimalDaysInFirstWeek() > for Windows has been implemented to suffice the bug claim. Looks good to me. Some minor comments below. src/java.base/windows/classes/sun/util/locale/

Re: RFR: 8257964: Broken Calendar#getMinimalDaysInFirstWeek with java.locale.providers=HOST [v2]

2020-12-10 Thread Joe Wang
On Fri, 11 Dec 2020 01:25:48 GMT, Naoto Sato wrote: >> src/java.base/windows/classes/sun/util/locale/provider/HostLocaleProviderAdapterImpl.java >> line 78: >> >>> 76: // CalendarData value types >>> 77: private static final int CD_FIRSTDAYOFWEEK = 0; >>> 78: private static final in

Re: RFR: 8257964: Broken Calendar#getMinimalDaysInFirstWeek with java.locale.providers=HOST [v2]

2020-12-11 Thread Joe Wang
On Fri, 11 Dec 2020 01:28:59 GMT, Naoto Sato wrote: >> test/jdk/java/util/Locale/LocaleProvidersRun.java line 177: >> >>> 175: >>> 176: //testing 8257964 fix. (macOS/Windows only) >>> 177: testRun("HOST", "bug8257964Test", "", "", ""); >> >> This test runs only if the platform

Re: RFR: 8253497: Core Libs Terminology Refresh

2020-12-14 Thread Joe Wang
On Mon, 14 Dec 2020 19:36:48 GMT, Brent Christian wrote: > This is part of an effort in the JDK to replace archaic/non-inclusive words > with more neutral terms (see JDK-8253315 for details). > > Here are the changes covering core libraries code and tests. Terms were > changed as follows: > 1

Re: RFR: 8253497: Core Libs Terminology Refresh [v2]

2020-12-14 Thread Joe Wang
On Tue, 15 Dec 2020 01:36:27 GMT, Brent Christian wrote: >> src/java.management.rmi/share/classes/javax/management/remote/rmi/RMIConnectorServer.java >> line 152: >> >>> 150: * >>> 151: * Care must be taken when defining such a filter, as defining >>> 152: * an accept-list too r

Re: [jdk16] RFR: 8259075: Update the copyright notice in the files generated by CLDR Converter tool

2021-01-05 Thread Joe Wang
On Tue, 5 Jan 2021 17:59:01 GMT, Naoto Sato wrote: > Please review this copyright notice update in the CLDR Converter tool. It is > now synched with src/java.base/share/legal/cldr.md file. Marked as reviewed by joehw (Reviewer). - PR: https://git.openjdk.java.net/jdk16/pull/85

Re: RFR: 8259528: Broken Link for [java.text.Normalizer.Form]

2021-01-11 Thread Joe Wang
On Mon, 11 Jan 2021 16:54:53 GMT, Naoto Sato wrote: > Please review this simple doc fix. Oops, forgot to submit the review ;-) src/java.base/share/classes/java/text/Normalizer.java line 48: > 46: * The {@code normalize} method supports the standard normalization forms > 47: * described in >

Re: RFR: 8259528: Broken Link for [java.text.Normalizer.Form]

2021-01-11 Thread Joe Wang
On Mon, 11 Jan 2021 16:54:53 GMT, Naoto Sato wrote: > Please review this simple doc fix. Marked as reviewed by joehw (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/2028

Re: RFR: 8261179: Norwegian Bokmål Locale fallback issue

2021-02-04 Thread Joe Wang
On Thu, 4 Feb 2021 22:17:12 GMT, Naoto Sato wrote: > Please review this fix. The bug was revealed while porting CLDR v39 into the > JDK, where they switch the Norwegian Bokmal from "nb" to "no". [1] Some > regression tests started failing with it because COMPAT provider's "nb" > resource was u

Re: RFR: 8261279: sun/util/resources/cldr/TimeZoneNamesTest.java timed out

2021-02-08 Thread Joe Wang
On Mon, 8 Feb 2021 21:52:45 GMT, Lance Andersen wrote: >> Please review this simple test case fix. By sampling locales to test, it >> reduces the possibility of the time out. > > Marked as reviewed by lancea (Reviewer). The fix looks ok to me. Just an interesting note that this test took such a

Re: RFR: 8261279: sun/util/resources/cldr/TimeZoneNamesTest.java timed out

2021-02-08 Thread Joe Wang
On Mon, 8 Feb 2021 21:42:36 GMT, Naoto Sato wrote: > Please review this simple test case fix. By sampling locales to test, it > reduces the possibility of the time out. Marked as reviewed by joehw (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/2465

Re: RFR: 8261279: sun/util/resources/cldr/TimeZoneNamesTest.java timed out

2021-02-08 Thread Joe Wang
On Tue, 9 Feb 2021 00:04:44 GMT, Naoto Sato wrote: >> The fix looks ok to me. Just an interesting note that this test took such a >> long time to run. Given the amount of locales, limit(30) would reduce the >> time to approximately 1/5 of the time, that would still be about 100,000ms >> accord

Re: RFR: 8261621: Delegate Unicode history from JLS to j.l.Character [v2]

2021-02-11 Thread Joe Wang
On Fri, 12 Feb 2021 04:06:55 GMT, Naoto Sato wrote: >> Please review this doc fix to j.l.Character, which now includes the table of >> the history of supported Unicode versions. A corresponding CSR will be filed >> accordingly. > > Naoto Sato has updated the pull request incrementally with one

Re: RFR: 8261919: java/util/Locale/LocaleProvidersRun.java failed with "RuntimeException: Expected log was not emitted. LogRecord: null"

2021-02-22 Thread Joe Wang
On Tue, 23 Feb 2021 02:09:01 GMT, Naoto Sato wrote: > Please review the fix to this test case failure that occurs with the usage > tracker enabled JRE. Marked as reviewed by joehw (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/2683

Re: RFR: 8263890: Broken links to Unicode.org

2021-03-19 Thread Joe Wang
On Fri, 19 Mar 2021 17:57:31 GMT, Naoto Sato wrote: > Fixed several broken links to Unicode.org. Some minor comments. src/java.base/share/classes/java/text/Collator.java line 211: > 209: * FULL_DECOMPOSITION corresponds to Normalization Form KD as > 210: * described in > 211: *

Re: RFR: 8263890: Broken links to Unicode.org [v2]

2021-03-19 Thread Joe Wang
On Fri, 19 Mar 2021 21:23:03 GMT, Naoto Sato wrote: >> Fixed several broken links to Unicode.org. > > Naoto Sato has updated the pull request incrementally with one additional > commit since the last revision: > > Addressed review comments. Looks all good. Thanks Naoto. - Marke

  1   2   >