Re: [14] RFR: 8212970: TZ database in "vanguard" format support

2019-07-29 Thread Roger Riggs
Hi Naoto, Thanks for the explanation. Looks good. Roger On 7/25/19 4:04 PM, naoto.s...@oracle.com wrote: Hi Roger, On 7/25/19 7:47 AM, Roger Riggs wrote: Hi Naoto, TestZoneInfo310.java: With the composition of the tzdir path up and over to the make directory for the tzdir it might be

Re: [14] RFR: 8212970: TZ database in "vanguard" format support

2019-07-29 Thread Stephen Colebourne
Thanks for these changes, +1 Stephen On Tue, 23 Jul 2019 at 23:18, wrote: > > Hi, > > Please review the fix to the following enhancement: > > https://bugs.openjdk.java.net/browse/JDK-8212970 > > The proposed changeset is located at: > > https://cr.openjdk.java.net/~naoto/8212970/webrev.09/ > >

Re: [14] RFR: 8212970: TZ database in "vanguard" format support

2019-07-25 Thread Joe Wang
Looks all good Naoto :-) -Joe On 7/25/19 2:35 PM, naoto.s...@oracle.com wrote: Hi Joe, Yes, I only removed not in-use files, i.e., 2 & 4. I sent the previous email just after confirming that all tests (open/closed) passed on a platform :-) Naoto On 7/25/19 2:24 PM, Joe Wang wrote: Hi

Re: [14] RFR: 8212970: TZ database in "vanguard" format support

2019-07-25 Thread naoto . sato
Hi Joe, Yes, I only removed not in-use files, i.e., 2 & 4. I sent the previous email just after confirming that all tests (open/closed) passed on a platform :-) Naoto On 7/25/19 2:24 PM, Joe Wang wrote: Hi Naoto, The legacy trap :-) Relevant files: 1. make/data/tzdata/jdk11_backward 2.

Re: [14] RFR: 8212970: TZ database in "vanguard" format support

2019-07-25 Thread Joe Wang
Hi Naoto, The legacy trap :-) Relevant files: 1. make/data/tzdata/jdk11_backward 2. test/jdk/sun/util/calendar/zi/tzdata/jdk11_backward 3. test/jdk/sun/util/calendar/zi/tzdata_jdk/jdk11_backward 4. test/jdk/sun/util/calendar/zi/tzdata_jdk/jdk11_full_backward I see you reverted changes to (1)

Re: [14] RFR: 8212970: TZ database in "vanguard" format support

2019-07-25 Thread naoto . sato
Hi Roger, On 7/25/19 7:47 AM, Roger Riggs wrote: Hi Naoto, TestZoneInfo310.java: With the composition of the tzdir path up and over to the make directory for the tzdir it might be useful to do an explicit check that the directory exists. That way if the directory structure on the build side

Re: [14] RFR: 8212970: TZ database in "vanguard" format support

2019-07-25 Thread Erik Joelsson
Build change looks good. /Erik On 2019-07-24 15:24, naoto.s...@oracle.com wrote: Hi Joe, Thank you for the review. On 7/24/19 2:57 PM, Joe Wang wrote: Hi Naoto, The method findNegativeSavings method in TzdbZoneRulesProvider.java states that it "Find the minimum negative savings". While

Re: [14] RFR: 8212970: TZ database in "vanguard" format support

2019-07-25 Thread Roger Riggs
Hi Naoto, TestZoneInfo310.java: With the composition of the tzdir path up and over to the make directory for the tzdir it might be useful to do an explicit check that the directory exists. That way if the directory structure on the build side changes, there will be a test failure makine it

Re: [14] RFR: 8212970: TZ database in "vanguard" format support

2019-07-24 Thread Joe Wang
Thanks Naoto.  Looks good. -Joe On 7/24/19 3:24 PM, naoto.s...@oracle.com wrote: Hi Joe, Thank you for the review. On 7/24/19 2:57 PM, Joe Wang wrote: Hi Naoto, The method findNegativeSavings method in TzdbZoneRulesProvider.java states that it "Find the minimum negative savings". While

Re: [14] RFR: 8212970: TZ database in "vanguard" format support

2019-07-24 Thread naoto . sato
Hi Joe, Thank you for the review. On 7/24/19 2:57 PM, Joe Wang wrote: Hi Naoto, The method findNegativeSavings method in TzdbZoneRulesProvider.java states that it "Find the minimum negative savings". While the result is correct since the rules all have the same value for SAVE, I wonder if

Re: [14] RFR: 8212970: TZ database in "vanguard" format support

2019-07-24 Thread Joe Wang
Hi Naoto, The method findNegativeSavings method in TzdbZoneRulesProvider.java states that it "Find the minimum negative savings". While the result is correct since the rules all have the same value for SAVE, I wonder if that's ideal conceptually. Given a start LDT, shouldn't it be looking

Re: [14] RFR: 8212970: TZ database in "vanguard" format support

2019-07-24 Thread naoto . sato
Hi Roger, On 7/24/19 1:09 PM, naoto.s...@oracle.com wrote: Thanks for the review, Roger. On 7/24/19 12:47 PM, Roger Riggs wrote: Hi Naoto, Adjusting the data during import looks fine. Typo: TzdbZoneRulesProvider.java:504  "ususally" -> "usually" Will fix it. Removing the source

Re: [14] RFR: 8212970: TZ database in "vanguard" format support

2019-07-24 Thread naoto . sato
Thanks for the review, Roger. On 7/24/19 12:47 PM, Roger Riggs wrote: Hi Naoto, Adjusting the data during import looks fine. Typo: TzdbZoneRulesProvider.java:504  "ususally" -> "usually" Will fix it. Removing the source duplication is good. Is there a way to remove the duplication of

Re: [14] RFR: 8212970: TZ database in "vanguard" format support

2019-07-24 Thread Roger Riggs
Hi Naoto, Adjusting the data during import looks fine. Typo: TzdbZoneRulesProvider.java:504  "ususally" -> "usually" Removing the source duplication is good. Is there a way to remove the duplication of the TZDATA files themselves? make/data/tzdata/* and test/jdk/sun/til/calendar/zi/* Looks

[14] RFR: 8212970: TZ database in "vanguard" format support

2019-07-23 Thread naoto . sato
Hi, Please review the fix to the following enhancement: https://bugs.openjdk.java.net/browse/JDK-8212970 The proposed changeset is located at: https://cr.openjdk.java.net/~naoto/8212970/webrev.09/ This change aims to support the "vanguard" IANA time zone data format, which uses the negative