Re: [14] RFR: 8230136: DateTimeFormatterBuilder.FractionPrinterParser#parse fails to verify minWidth
+1 On 9/11/19 8:48 AM, Stephen Colebourne wrote: +1 On Wed, 11 Sep 2019 at 13:38, wrote: Hi Stephen, I confirmed that issuing parseStrict() was irrelevant. The incorrect text won't throw the exception in "lenient" mode as well. In addition, "builder" is always instantiated in AbstractTestPrinterParser on each TestNG invocation and "strict" is the default mode. Thus I did not explicitly issue parseStrict() in the test. BTW, I found typos wrt the default parse mode. Included the corrections in this webrev as well: http://cr.openjdk.java.net/~naoto/8230136/webrev.01/ Naoto On 9/11/19 2:01 AM, Stephen Colebourne wrote: The bug references parseStrict() but the test does not. Is the builder already set to parseStrict() ? Anyway, if the bug is to be clearly squished, parseStrict() should appear somewhere. Stephen On Tue, 10 Sep 2019 at 23:42, Joe Wang wrote: +1, looks good to me. Best regards, Joe On 9/10/19 2:20 PM, naoto.s...@oracle.com wrote: Hello, Please review the fix to the following issue: https://bugs.openjdk.java.net/browse/JDK-8230136 The proposed changeset is located at: http://cr.openjdk.java.net/~naoto/8230136/webrev.00/ The fix is to correct the condition of the invalid case, as suggested in the bug report. Naoto
Re: [14] RFR: 8230136: DateTimeFormatterBuilder.FractionPrinterParser#parse fails to verify minWidth
+1 On Wed, 11 Sep 2019 at 13:38, wrote: > > Hi Stephen, > > I confirmed that issuing parseStrict() was irrelevant. The incorrect > text won't throw the exception in "lenient" mode as well. In addition, > "builder" is always instantiated in AbstractTestPrinterParser on each > TestNG invocation and "strict" is the default mode. Thus I did not > explicitly issue parseStrict() in the test. > > BTW, I found typos wrt the default parse mode. Included the corrections > in this webrev as well: > > http://cr.openjdk.java.net/~naoto/8230136/webrev.01/ > > Naoto > > On 9/11/19 2:01 AM, Stephen Colebourne wrote: > > The bug references parseStrict() but the test does not. Is the builder > > already set to parseStrict() ? Anyway, if the bug is to be clearly > > squished, parseStrict() should appear somewhere. > > Stephen > > > > On Tue, 10 Sep 2019 at 23:42, Joe Wang wrote: > >> > >> +1, looks good to me. > >> > >> Best regards, > >> Joe > >> > >> On 9/10/19 2:20 PM, naoto.s...@oracle.com wrote: > >>> Hello, > >>> > >>> Please review the fix to the following issue: > >>> > >>> https://bugs.openjdk.java.net/browse/JDK-8230136 > >>> > >>> The proposed changeset is located at: > >>> > >>> http://cr.openjdk.java.net/~naoto/8230136/webrev.00/ > >>> > >>> The fix is to correct the condition of the invalid case, as suggested > >>> in the bug report. > >>> > >>> Naoto > >>
Re: [14] RFR: 8230136: DateTimeFormatterBuilder.FractionPrinterParser#parse fails to verify minWidth
Hi Stephen, I confirmed that issuing parseStrict() was irrelevant. The incorrect text won't throw the exception in "lenient" mode as well. In addition, "builder" is always instantiated in AbstractTestPrinterParser on each TestNG invocation and "strict" is the default mode. Thus I did not explicitly issue parseStrict() in the test. BTW, I found typos wrt the default parse mode. Included the corrections in this webrev as well: http://cr.openjdk.java.net/~naoto/8230136/webrev.01/ Naoto On 9/11/19 2:01 AM, Stephen Colebourne wrote: The bug references parseStrict() but the test does not. Is the builder already set to parseStrict() ? Anyway, if the bug is to be clearly squished, parseStrict() should appear somewhere. Stephen On Tue, 10 Sep 2019 at 23:42, Joe Wang wrote: +1, looks good to me. Best regards, Joe On 9/10/19 2:20 PM, naoto.s...@oracle.com wrote: Hello, Please review the fix to the following issue: https://bugs.openjdk.java.net/browse/JDK-8230136 The proposed changeset is located at: http://cr.openjdk.java.net/~naoto/8230136/webrev.00/ The fix is to correct the condition of the invalid case, as suggested in the bug report. Naoto
Re: [14] RFR: 8230136: DateTimeFormatterBuilder.FractionPrinterParser#parse fails to verify minWidth
The bug references parseStrict() but the test does not. Is the builder already set to parseStrict() ? Anyway, if the bug is to be clearly squished, parseStrict() should appear somewhere. Stephen On Tue, 10 Sep 2019 at 23:42, Joe Wang wrote: > > +1, looks good to me. > > Best regards, > Joe > > On 9/10/19 2:20 PM, naoto.s...@oracle.com wrote: > > Hello, > > > > Please review the fix to the following issue: > > > > https://bugs.openjdk.java.net/browse/JDK-8230136 > > > > The proposed changeset is located at: > > > > http://cr.openjdk.java.net/~naoto/8230136/webrev.00/ > > > > The fix is to correct the condition of the invalid case, as suggested > > in the bug report. > > > > Naoto >
Re: [14] RFR: 8230136: DateTimeFormatterBuilder.FractionPrinterParser#parse fails to verify minWidth
+1, looks good to me. Best regards, Joe On 9/10/19 2:20 PM, naoto.s...@oracle.com wrote: Hello, Please review the fix to the following issue: https://bugs.openjdk.java.net/browse/JDK-8230136 The proposed changeset is located at: http://cr.openjdk.java.net/~naoto/8230136/webrev.00/ The fix is to correct the condition of the invalid case, as suggested in the bug report. Naoto
[14] RFR: 8230136: DateTimeFormatterBuilder.FractionPrinterParser#parse fails to verify minWidth
Hello, Please review the fix to the following issue: https://bugs.openjdk.java.net/browse/JDK-8230136 The proposed changeset is located at: http://cr.openjdk.java.net/~naoto/8230136/webrev.00/ The fix is to correct the condition of the invalid case, as suggested in the bug report. Naoto
