Re: RFR 8235238: Parsing a time string ignores any custom TimeZoneNameProvider
+1 On 12/12/19 7:38 PM, Joe Wang wrote: +1 -Joe On 12/12/19 2:07 PM, naoto.s...@oracle.com wrote: Sorry, I seem to have posted an old webrev, which included unnecessary retrieval of the generic name (4168-4173 in v.00). Here is the correct webrev: http://cr.openjdk.java.net/~naoto/8235238/webrev.01/ Naoto On 12/12/19 1:37 PM, 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 you for the review. The original code loops through zoneStrings array, and if the id exists in regionIds, then adds their display names in the tree (4142-4154). This process is not altered with my change. My change made regionIds mutable (line 4127) so that after the loop, it only contains custom ids by calling remove() (line 4144). Thus the new added block will only retrieve names for custom ids. Or am I missing something in your comment? Ok, that's what I was looking for. Thanks for the explanation. 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 process didn't need to loop through regionIds to add display names. Thanks, Joe On 12/11/19 1:21 PM, naoto.s...@oracle.com wrote: Hi, Please review the fix for the following issue: https://bugs.openjdk.java.net/browse/JDK-8235238 The proposed changeset is located at: https://cr.openjdk.java.net/~naoto/8235238/webrev.00/ The fix will retrieve the custom zone names from the time zone name provider, for the custom ZoneRulesProvider implementations. Naoto
Re: RFR 8235238: Parsing a time string ignores any custom TimeZoneNameProvider
+1 -Joe On 12/12/19 2:07 PM, naoto.s...@oracle.com wrote: Sorry, I seem to have posted an old webrev, which included unnecessary retrieval of the generic name (4168-4173 in v.00). Here is the correct webrev: http://cr.openjdk.java.net/~naoto/8235238/webrev.01/ Naoto On 12/12/19 1:37 PM, 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 you for the review. The original code loops through zoneStrings array, and if the id exists in regionIds, then adds their display names in the tree (4142-4154). This process is not altered with my change. My change made regionIds mutable (line 4127) so that after the loop, it only contains custom ids by calling remove() (line 4144). Thus the new added block will only retrieve names for custom ids. Or am I missing something in your comment? Ok, that's what I was looking for. Thanks for the explanation. 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 process didn't need to loop through regionIds to add display names. Thanks, Joe On 12/11/19 1:21 PM, naoto.s...@oracle.com wrote: Hi, Please review the fix for the following issue: https://bugs.openjdk.java.net/browse/JDK-8235238 The proposed changeset is located at: https://cr.openjdk.java.net/~naoto/8235238/webrev.00/ The fix will retrieve the custom zone names from the time zone name provider, for the custom ZoneRulesProvider implementations. Naoto
Re: RFR 8235238: Parsing a time string ignores any custom TimeZoneNameProvider
Sorry, I seem to have posted an old webrev, which included unnecessary retrieval of the generic name (4168-4173 in v.00). Here is the correct webrev: http://cr.openjdk.java.net/~naoto/8235238/webrev.01/ Naoto On 12/12/19 1:37 PM, 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 you for the review. The original code loops through zoneStrings array, and if the id exists in regionIds, then adds their display names in the tree (4142-4154). This process is not altered with my change. My change made regionIds mutable (line 4127) so that after the loop, it only contains custom ids by calling remove() (line 4144). Thus the new added block will only retrieve names for custom ids. Or am I missing something in your comment? Ok, that's what I was looking for. Thanks for the explanation. 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 process didn't need to loop through regionIds to add display names. Thanks, Joe On 12/11/19 1:21 PM, naoto.s...@oracle.com wrote: Hi, Please review the fix for the following issue: https://bugs.openjdk.java.net/browse/JDK-8235238 The proposed changeset is located at: https://cr.openjdk.java.net/~naoto/8235238/webrev.00/ The fix will retrieve the custom zone names from the time zone name provider, for the custom ZoneRulesProvider implementations. Naoto
Re: RFR 8235238: Parsing a time string ignores any custom TimeZoneNameProvider
+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 you for the review. The original code loops through zoneStrings array, and if the id exists in regionIds, then adds their display names in the tree (4142-4154). This process is not altered with my change. My change made regionIds mutable (line 4127) so that after the loop, it only contains custom ids by calling remove() (line 4144). Thus the new added block will only retrieve names for custom ids. Or am I missing something in your comment? Ok, that's what I was looking for. Thanks for the explanation. 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 process didn't need to loop through regionIds to add display names. Thanks, Joe On 12/11/19 1:21 PM, naoto.s...@oracle.com wrote: Hi, Please review the fix for the following issue: https://bugs.openjdk.java.net/browse/JDK-8235238 The proposed changeset is located at: https://cr.openjdk.java.net/~naoto/8235238/webrev.00/ The fix will retrieve the custom zone names from the time zone name provider, for the custom ZoneRulesProvider implementations. Naoto
Re: RFR 8235238: Parsing a time string ignores any custom TimeZoneNameProvider
On 12/12/19 12:31 PM, naoto.s...@oracle.com wrote: Hi Joe, Thank you for the review. The original code loops through zoneStrings array, and if the id exists in regionIds, then adds their display names in the tree (4142-4154). This process is not altered with my change. My change made regionIds mutable (line 4127) so that after the loop, it only contains custom ids by calling remove() (line 4144). Thus the new added block will only retrieve names for custom ids. Or am I missing something in your comment? Ok, that's what I was looking for. Thanks for the explanation. 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 process didn't need to loop through regionIds to add display names. Thanks, Joe On 12/11/19 1:21 PM, naoto.s...@oracle.com wrote: Hi, Please review the fix for the following issue: https://bugs.openjdk.java.net/browse/JDK-8235238 The proposed changeset is located at: https://cr.openjdk.java.net/~naoto/8235238/webrev.00/ The fix will retrieve the custom zone names from the time zone name provider, for the custom ZoneRulesProvider implementations. Naoto
Re: RFR 8235238: Parsing a time string ignores any custom TimeZoneNameProvider
Hi Joe, Thank you for the review. The original code loops through zoneStrings array, and if the id exists in regionIds, then adds their display names in the tree (4142-4154). This process is not altered with my change. My change made regionIds mutable (line 4127) so that after the loop, it only contains custom ids by calling remove() (line 4144). Thus the new added block will only retrieve names for custom ids. Or am I missing something in your comment? 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 process didn't need to loop through regionIds to add display names. Thanks, Joe On 12/11/19 1:21 PM, naoto.s...@oracle.com wrote: Hi, Please review the fix for the following issue: https://bugs.openjdk.java.net/browse/JDK-8235238 The proposed changeset is located at: https://cr.openjdk.java.net/~naoto/8235238/webrev.00/ The fix will retrieve the custom zone names from the time zone name provider, for the custom ZoneRulesProvider implementations. Naoto
Re: RFR 8235238: Parsing a time string ignores any custom TimeZoneNameProvider
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:21 PM, naoto.s...@oracle.com wrote: Hi, Please review the fix for the following issue: https://bugs.openjdk.java.net/browse/JDK-8235238 The proposed changeset is located at: https://cr.openjdk.java.net/~naoto/8235238/webrev.00/ The fix will retrieve the custom zone names from the time zone name provider, for the custom ZoneRulesProvider implementations. Naoto
RFR 8235238: Parsing a time string ignores any custom TimeZoneNameProvider
Hi, Please review the fix for the following issue: https://bugs.openjdk.java.net/browse/JDK-8235238 The proposed changeset is located at: https://cr.openjdk.java.net/~naoto/8235238/webrev.00/ The fix will retrieve the custom zone names from the time zone name provider, for the custom ZoneRulesProvider implementations. Naoto
