Re: RFR: 8256515: javax.xml.XMLEventReader produces incorrect START_DOCUMENT event

2020-12-03 Thread Joe Wang
On Thu, 19 Nov 2020 16:34:36 GMT, Marius Volkhart wrote: >> @MariusVolkhart I'll work with you getting the fix in and get the test >> running using jtreg. Give me a few moments to reproduce. > > Thanks @jerboaa > > I have a JBS number now: >

Re: RFR: 8256839: JavaDoc for java.time.Period.negated() method

2020-11-23 Thread Joe Wang
On Mon, 23 Nov 2020 19:24:51 GMT, Naoto Sato wrote: > Hi, > > Please review this simple doc fix. Thanks. Marked as reviewed by joehw (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/1399

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:

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: Filtered XMLStreamReader Exceptions (java.xml)

2020-11-05 Thread Joe Wang
On Wed, Oct 28, 2020 at 12:52 PM Joe Wang <mailto:huizhe.w...@oracle.com>> wrote: Hi Mike, As you said, creating a bug report would be a good start. If it involves a signature change, it'd need to go through a proper review (CSR) process. When you are ready to s

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: 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: 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: Filtered XMLStreamReader Exceptions (java.xml)

2020-10-28 Thread Joe Wang
Hi Mike, As you said, creating a bug report would be a good start. If it involves a signature change, it'd need to go through a proper review (CSR) process. When you are ready to submit a bug report, please make sure to add a test case to illustrate the use case scenario. Thanks, Joe On

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

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. > >

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

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

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

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

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

Integrated: 8251495: Remove the implNote in the DOM package description added by JDK-8249643

2020-09-15 Thread Joe Wang
On Wed, 9 Sep 2020 22:56:14 GMT, Joe Wang wrote: > Revert changes made by JDK-8249643, removing the implNote. This pull request has now been integrated. Changeset: 5191f315 Author: Joe Wang URL: https://git.openjdk.java.net/jdk/commit/5191f315 Stats: 25 lines in 1 file chan

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

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: 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

Re: RFR: 8251495: Clarify DOM documentation

2020-09-10 Thread Joe Wang
On Thu, 10 Sep 2020 16:16:57 GMT, Joe Wang wrote: >> Marked as reviewed by alanb (Reviewer). > > Remove the implNote from the package description added by JDK-8249643. > The implementation's deviation from the specification was actually wrong. > It incorrectly identified chara

Re: RFR: 8251495: Clarify DOM documentation

2020-09-10 Thread Joe Wang
On Thu, 10 Sep 2020 05:45:59 GMT, Alan Bateman wrote: >> Revert changes made by JDK-8249643, removing the implNote. > > Marked as reviewed by alanb (Reviewer). Remove the implNote from the package description added by JDK-8249643. The implementation's deviation from the specification was

Re: RFR: 8251495: Clarify DOM documentation

2020-09-09 Thread Joe Wang
On Wed, 9 Sep 2020 23:15:58 GMT, Lance Andersen wrote: >> Revert changes made by JDK-8249643, removing the implNote. > > Based on the spec review and follow-on discussions, this change makes sense An implNote was added to the DOM's package description in an attempt to document the JDK

RFR: 8251495: Clarify DOM documentation

2020-09-09 Thread Joe Wang
Revert changes made by JDK-8249643, removing the implNote. - Commit messages: - 8251495: Clarify DOM documentation Changes: https://git.openjdk.java.net/jdk/pull/100/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=100=00 Issue:

Re: RFR: 8252740: java/util/Properties/LoadAndStoreXMLWithDefaults.java fails after JDK-8252354

2020-09-03 Thread Joe Wang
On 9/3/20 2:51 AM, Alan Bateman wrote: On 03/09/2020 10:22, jiefu(傅杰) wrote: Hi all, JBS:    https://bugs.openjdk.java.net/browse/JDK-8252740 Webrev: http://cr.openjdk.java.net/~jiefu/8252740/webrev.00/ After JDK-8252354, properties whose keys or values are none strings won't be skipped

RFR [16/java.xml] 8252354 : Properties :: storeToXML method does not throw ClassCastException when supplied non strings.

2020-09-01 Thread Joe Wang
Please review a fix to report CCE when keys or values are not Strings as specified. JBS: https://bugs.openjdk.java.net/browse/JDK-8252354 webrev: http://cr.openjdk.java.net/~joehw/jdk16/8252354/webrev/ Thanks, Joe

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

Re: RFR [16/java.xml] 8251561: Fix doclint warnings in the java.xml package

2020-08-26 Thread Joe Wang
On 8/26/20 4:00 AM, Alan Bateman wrote: On 21/08/2020 19:23, Joe Wang wrote: Pelase review a patch to add the missing @return, @throws, @param statements in the java.xml package (excluding the DOM component). JBS: https://bugs.openjdk.java.net/browse/JDK-8251561 CSR: https

Re: RFR [16/java.xml] 8251561: Fix doclint warnings in the java.xml package

2020-08-25 Thread Joe Wang
categories by default? /Erik On 2020-08-25 11:47, Joe Wang wrote: Cc-ing build-...@openjdk.java.net (makefile change: make/Docs.gmk) Updated webrev: http://cr.openjdk.java.net/~joehw/jdk16/8251561/webrev_04/ Thanks Roger! Please see inline comments. On 8/25/20 8:09 AM, Roger Riggs wrote: Hi Joe

Re: RFR [16/java.xml] 8251561: Fix doclint warnings in the java.xml package

2020-08-25 Thread Joe Wang
, using @code around true, false, missing periods at the end of first sentences, etc.  But that's a different task. Created a bug to keep track of this: https://bugs.openjdk.java.net/browse/JDK-8252328 Thanks, Joe On 8/24/20 5:44 PM, Joe Wang wrote: Hi all,  adding Roger's commen

Re: RFR [16/java.xml] 8251561: Fix doclint warnings in the java.xml package

2020-08-24 Thread Joe Wang
://cr.openjdk.java.net/~joehw/jdk16/8251561/webrev_03/ -Joe On 8/24/20 3:27 PM, Lance Andersen wrote: Looks OK Joe On Aug 24, 2020, at 5:44 PM, Joe Wang <mailto:huizhe.w...@oracle.com>> wrote: Hi all,  adding Roger's comment for the make file to webrev_02 (the only change to webrev_01 is

Re: RFR [16/java.xml] 8251561: Fix doclint warnings in the java.xml package

2020-08-24 Thread Joe Wang
, This looks OK. On Aug 21, 2020, at 2:23 PM, Joe Wang wrote: Pelase review a patch to add the missing @return, @throws, @param statements in the java.xml package (excluding the DOM component). JBS: https://bugs.openjdk.java.net/browse/JDK-8251561 CSR: https://bugs.openjdk.java.net/browse/JDK

RFR [16/java.xml] 8251561: Fix doclint warnings in the java.xml package

2020-08-21 Thread Joe Wang
Pelase review a patch to add the missing @return, @throws, @param statements in the java.xml package (excluding the DOM component). JBS: https://bugs.openjdk.java.net/browse/JDK-8251561 CSR: https://bugs.openjdk.java.net/browse/JDK-8251995 webrev:

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

2020-08-18 Thread Joe Wang
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 method

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

2020-08-18 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 to me

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

Re: RFR 8251208: Fix javadoc warnings in java.sql and java.sql.rowsets

2020-08-14 Thread Joe Wang
the typos (missed I guess due to lack of coffee this am ;-) ) http://cr.openjdk.java.net/~lancea/8251208/webrev.01/index.html is the updated webrev Best Lance On Aug 14, 2020, at 1:20 PM, Joe Wang <mailto:huizhe.w...@oracle.com>> wrote: Hi Lance, Looks good to me overall. * **Mi

Re: RFR 8251208: Fix javadoc warnings in java.sql and java.sql.rowsets

2020-08-14 Thread Joe Wang
Hi Lance, Looks good to me overall. * **Minor typos in the CSR:* Address the Fix "no comment" warnings in java.sql and java.sql.rowsetsgenerated by javadoc -Xdoclint     ^ remove Fix  ^ missing a spacebetween rowsetsgenerated java.sql and

Re: RFR [16/java.xml] 8246816: XMLGregorianCalendar.hashCode() produces far too many identical hashes

2020-08-07 Thread Joe Wang
. Otherwise, looks good. Roger On 8/7/20 3:13 PM, Joe Wang wrote: Hi Naoto, Why not :-). I kept it to go with equals(). Removing both now. Added a reference comparison as did in the Impl class. http://cr.openjdk.java.net/~joehw/jdk16/8246816/webrev_04/ Joe On 8/7/20 10:50 AM, naoto.s

Re: RFR [16/java.xml] 8246816: XMLGregorianCalendar.hashCode() produces far too many identical hashes

2020-08-07 Thread Joe Wang
Thanks Naoto! Joe On 8/7/20 12:45 PM, naoto.s...@oracle.com wrote: Looks good. Thank you for the change. Naoto On 8/7/20 12:13 PM, Joe Wang wrote: Hi Naoto, Why not :-). I kept it to go with equals(). Removing both now. Added a reference comparison as did in the Impl class. http

Re: RFR [16/java.xml] 8246816: XMLGregorianCalendar.hashCode() produces far too many identical hashes

2020-08-07 Thread Joe Wang
XMLGregorianCalendarImpl.hashCode() just calls super.hashCode(), could it be removed entirely? Also XMLGregorianCalendarImpl.equals() seems to do the same thing as its parent. Could it also be removed? Naoto On 8/7/20 10:16 AM, Joe Wang wrote: Naoto, Roger, Sorry have to come back with another iteration. It looks like

Re: RFR [16/java.xml] 8246816: XMLGregorianCalendar.hashCode() produces far too many identical hashes

2020-08-07 Thread Joe Wang
arImpl and XMLGregorianCalendar. webrev_03: http://cr.openjdk.java.net/~joehw/jdk16/8246816/webrev_03/ Thanks, Joe On 8/6/20 1:51 PM, Joe Wang wrote: Thanks Naoto, Roger!  I'll prepare push based on version 01. Best, Joe On 8/6/20 1:29 PM, Roger Riggs wrote: +1 On 8/6/20 4:18 PM, naoto.s...@orac

Re: RFR [16/java.xml] 8246816: XMLGregorianCalendar.hashCode() produces far too many identical hashes

2020-08-06 Thread Joe Wang
in two objects. So fine with the version 01. Naoto On 8/6/20 12:18 PM, Joe Wang wrote: Thanks Naoto, Roger for the review! For Naoto's concern about the equals method using EonAndYear and FractionalSecond, I did cut corners using the all int getXXX method. The rational was: it fulfills

Re: RFR [16/java.xml] 8246816: XMLGregorianCalendar.hashCode() produces far too many identical hashes

2020-08-06 Thread Joe Wang
for the year? Naoto On 8/5/20 4:37 PM, Joe Wang wrote: Hello, Please review a fix for the hashCode generation. JBS: https://bugs.openjdk.java.net/browse/JDK-8246816 webrev: http://cr.openjdk.java.net/~joehw/jdk16/8246816/webrev/ Thanks, Joe

RFR [16/java.xml] 8246816: XMLGregorianCalendar.hashCode() produces far too many identical hashes

2020-08-05 Thread Joe Wang
Hello, Please review a fix for the hashCode generation. JBS: https://bugs.openjdk.java.net/browse/JDK-8246816 webrev: http://cr.openjdk.java.net/~joehw/jdk16/8246816/webrev/ Thanks, Joe

Re: RFR 8233048: WeekFields.ISO is not a singleton

2020-07-30 Thread Joe Wang
+1 -Joe On 7/30/20 2:53 PM, naoto.s...@oracle.com wrote: Hi, Please review the fix to the following issue: https://bugs.openjdk.java.net/browse/JDK-8233048 The proposed chageset is located at: https://cr.openjdk.java.net/~naoto/8233048/webrev.00/ java.time.temporal.WeekFields.ISO is

RFR [16/java.xml] 8250638: Address reliance on default constructors in java.xml

2020-07-30 Thread Joe Wang
Hello, Please review a change to remove reliance of default constructors in java.xml. JBS: https://bugs.openjdk.java.net/browse/JDK-8250638 CSR: https://bugs.openjdk.java.net/browse/JDK-8250800 Patch: diff --git a/src/java.xml/share/classes/org/xml/sax/HandlerBase.java

RFR [16/java.xml] 8249643: Clarify DOM documentation

2020-07-28 Thread Joe Wang
Hello, Please review a doc clarification for the org.w3c.dom package. JBS: https://bugs.openjdk.java.net/browse/JDK-8249643 CSR: https://bugs.openjdk.java.net/browse/JDK-8249904 webrev: http://cr.openjdk.java.net/~joehw/jdk16/8249643/webrev_02/ Thanks, Joe

Re: RFR: 8247546: Pattern matching does not skip correctly over supplementary characters

2020-07-28 Thread Joe Wang
Hi Naoto, The new webrev looks good. Best, Joe On 7/27/20 7:11 PM, naoto.s...@oracle.com wrote: Hi Joe, Thank you for the review! On 7/27/20 6:04 PM, Joe Wang wrote: Hi Naoto, Looks good to me, using the correct supplementary-aware implementation is an improvement for the impl to handle

Re: RFR: 8247546: Pattern matching does not skip correctly over supplementary characters

2020-07-27 Thread Joe Wang
Hi Naoto, Looks good to me, using the correct supplementary-aware implementation is an improvement for the impl to handle the situation. A note (probably a workaround) for the user's case, to replace invalid surrogate characters, he could have used Unicode escape sequences [\ud800-\udfff]

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

2020-07-22 Thread Joe Wang
:20 PM, Joe Wang wrote: 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 33

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

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

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

2020-07-20 Thread Joe Wang
/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 not true. Good point. But more i

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:

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

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 824767: Incorrect class name displayed in DriverManager trace output

2020-07-14 Thread Joe Wang
+1 Joe On 7/14/20 8:41 AM, Lance Andersen wrote: Hi all, Please review this trivial fix for https://bugs.openjdk.java.net/browse/JDK-8247677 , which addresses a couple of typos where the driver class name was not included in the trace

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: 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 covers

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[15/java.xml] Re: RFR [16/java.xml] 8248486: SafeThread illegal access to java.lang private fields should be removed

2020-07-13 Thread Joe Wang
Thanks Mandy. Pushed to the mainline/16 repo. -Joe On 7/13/20 10:13 AM, Mandy Chung wrote: Looks okay.  Same as Alan's comment,  I don't see there is any urgency to fix this in 15.   Fixing it in 16 is fine. Mandy On 7/12/20 3:21 PM, Joe Wang wrote: Hi all, Alan updated the bug

RFR[15/java.xml] Re: RFR [16/java.xml] 8248486: SafeThread illegal access to java.lang private fields should be removed

2020-07-12 Thread Joe Wang
Hi all, Alan updated the bug to indicate it fails since JDK 9. Given we still have a couple days for JDK 15, I've rebased the patch to the jdk15 repo and would like to check in the patch into JDK 15 instead (and let it be sync-ed to 16). Here's the webrev for JDK 15:

Re: RFR [16/java.xml] 8248348: Regression caused by the update to BCEL 6.0

2020-06-30 Thread Joe Wang
On 6/30/2020 7:44 PM, Stuart Marks wrote: An unconventional patch over an unusual hashcode/equals implementation is a bit too controversial. I'd like to propose a new patch that focus on the problem at hand, that is re-setting the opcode causes the item in HashSet to get lost because of

Re: RFR [16/java.xml] 8248348: Regression caused by the update to BCEL 6.0

2020-06-29 Thread Joe Wang
Hi all, (had a nice hike today, and asked the beautiful Lake 22 ;-)) An unconventional patch over an unusual hashcode/equals implementation is a bit too controversial. I'd like to propose a new patch that focus on the problem at hand, that is re-setting the opcode causes the item in HashSet

Re: RFR [16/java.xml] 8248348: Regression caused by the update to BCEL 6.0

2020-06-27 Thread Joe Wang
. In that case you can implement hashcode uncached and aligned with equals. (And not use it) Gruss Bernd -- http://bernd.eckenfels.net *Von:* Joe Wang *Gesendet:* Friday, June 26, 2020 8:29:41 AM *An:* Bernd Eckenfels ; Core-Libs

Re: RFR [16/java.xml] 8248348: Regression caused by the update to BCEL 6.0

2020-06-27 Thread Joe Wang
On 6/26/2020 12:46 PM, Stuart Marks wrote: On 6/25/20 4:53 PM, Joe Wang wrote: Please review a fix to a BCEL regression. At issue was the addition of hashCode() method to Instruction.java in BCEL 6.0. This hashCode() method was implemented to return the instruction's opcode

Re: RFR; [docs,15] JDK-8248060 small HTML issues in java.xml package-info.java files

2020-06-26 Thread Joe Wang
Looks good to me. The upstream SAX hasn't evolved for a long time (and there's no plan to do so). I think we're okay to make small changes. For the 2nd case, I think it's good enough since there are links to the classes at the beginning of the sentence -- a minor inconvenience (vs a straight

Re: RFR [16/java.xml] 8248348: Regression caused by the update to BCEL 6.0

2020-06-26 Thread Joe Wang
(o2) => o1.hashCode() == o2.hashCode(). I see equals() method delegates to a global InstructionComparator instance which can even change. Suppose it does not change. But what does such InstructionComparator compare? opcode (among other things)? Regards, Peter On Fri, 26 Jun 2020, 01:56 Jo

Re: RFR [16/java.xml] 8248348: Regression caused by the update to BCEL 6.0

2020-06-26 Thread Joe Wang
Von: core-libs-dev im Auftrag von Joe Wang Gesendet: Friday, June 26, 2020 1:53:08 AM An: Core-Libs-Dev Betreff: RFR [16/java.xml] 8248348: Regression caused by the update to BCEL 6.0 Hi, Please review a fix to a BCEL regression. At issue was the addition of hashCode

RFR [16/java.xml] 8248348: Regression caused by the update to BCEL 6.0

2020-06-25 Thread Joe Wang
Hi, Please review a fix to a BCEL regression. At issue was the addition of hashCode() method to Instruction.java in BCEL 6.0. This hashCode() method was implemented to return the instruction's opcode that unfortunately is mutable. The problem hasn't showed up until the code path led to calls

Re: RFR 15 8247521: (test) jdk/test/lib/hexdump/HexPrinterTest.java fails on windows

2020-06-15 Thread Joe Wang
+1 -Joe On 6/15/2020 7:09 AM, Roger Riggs wrote: Please review a test change of the test library HexPrinter. It should be comparing against the System.lineSeparator. diff --git a/test/lib-test/jdk/test/lib/hexdump/HexPrinterTest.java b/test/lib-test/jdk/test/lib/hexdump/HexPrinterTest.java

Re: RFR: 8245448: Remove minimum 4 digit requirement from Year.parse()

2020-06-12 Thread Joe Wang
+1.  The original poster was probably working on writing history (all the way back to before 1000), I assume ;-)  This enhancement would certainly make his job a bit easier. -Joe On 6/12/2020 11:17 AM, Roger Riggs wrote: Hi Naoto, Yes, looks good. Roger On 6/12/20 1:06 PM, Lance Andersen

Re: RFR 8207936: TestZipFile.java can fail with an OOM error

2020-06-10 Thread Joe Wang
Looks good to me, Lance. -Joe On 6/10/2020 2:47 PM, Lance Andersen wrote: Hi, Please review the fix for JDK-8207936, which addresses the OOM error when running the manual test TestZipFile.java. The fix also removes the call to "getMetaInfEntryNames” as this method was removed as part of

Re: [15] RFR: 8246662: Test java/time/test/java/time/format/TestUnicodeExtension.java failed on japanese locale.

2020-06-09 Thread Joe Wang
it is not depending on identity. Roger On 6/9/20 3:10 PM, naoto.s...@oracle.com wrote: Hi Joe, On 6/9/20 11:54 AM, Joe Wang wrote: Looks fine to me for the implementation of the method. Thank you for the review! It's interesting to observe that the with methods, as demonstrated

Re: RFR [15/java.xml] 8237456: Transform filtered through SAX filter mishandles character entities

2020-06-09 Thread Joe Wang
org.testng.Assert.*; — No need to change but typically which get suggested when you run Inspect Code Best Lance On Jun 9, 2020, at 1:54 AM, Joe Wang <mailto:huizhe.w...@oracle.com>> wrote: Hi, Please review a fix to a regression that resulted in invalid output in a transform that

Re: [15] RFR: 8246662: Test java/time/test/java/time/format/TestUnicodeExtension.java failed on japanese locale.

2020-06-09 Thread Joe Wang
On 6/9/2020 10:01 AM, naoto.s...@oracle.com wrote: Hi Roger, Thanks for the review. On 6/9/20 8:52 AM, Roger Riggs wrote: Hi Naoto, Since the default locale is being changed even briefly, the test should be run in /othervm. Add:  @run testng/othervm ... All

RFR [15/java.xml] 8237456: Transform filtered through SAX filter mishandles character entities

2020-06-08 Thread Joe Wang
Hi, Please review a fix to a regression that resulted in invalid output in a transform that contains entity references. The regression was caused by a JDK9 patch [1]. Adding back the conditional checks fixed the issue. JBS: https://bugs.openjdk.java.net/browse/JDK-8237456 webrevs:

Re: [15] RFR: 8246261: TCKLocalTime.java failed due to "AssertionError: expected [18:14:22] but found [18:14:23]"

2020-06-02 Thread Joe Wang
match or not does not matter in this test case, so I think we can just eliminate these exact match try loops. I will remove them and do some sniff testing on it. Naoto On 6/1/20 5:58 PM, Joe Wang wrote: Hi Naoto, The patch looks good to fix the failure. I'm just curious whether the 100-time

Re: [15] RFR: 8246261: TCKLocalTime.java failed due to "AssertionError: expected [18:14:22] but found [18:14:23]"

2020-06-01 Thread Joe Wang
Hi Naoto, The patch looks good to fix the failure. I'm just curious whether the 100-time comparison is necessary because of the existence of this assertion outside the loop that allowed the test to pass if the different was within a certain period of time. None of the tests had commented on

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: [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

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 to see

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

Re: RFR [15] 8245231 Javadoc for the readObject methods needs to be updated (8244342 followup)

2020-05-18 Thread Joe Wang
As it appears, I was missing a comma in the header as well (see https://bugs.openjdk.java.net/browse/JDK-8245238) The webrev is updated with the comma added (to PredicatedNodeTest.java) http://cr.openjdk.java.net/~joehw/jdk15/8245231/webrev/ Thanks, Joe On 5/18/2020 1:53 PM, Joe Wang wrote

Re: RFR [15] 8244342: Compilation warnings about unexpected serialization related method signatures.

2020-05-18 Thread Joe Wang
Thanks Mark. -Joe On 5/18/2020 1:49 PM, mark sheppard wrote: Hi Joe, all good I think.  regards Mark *From:* Joe Wang *Sent:* Monday 18 May 2020 19:36 *To:* mark sheppard ; core-libs-dev@openjdk.java.net *Cc

Re: RFR [15] 8244342: Compilation warnings about unexpected serialization related method signatures.

2020-05-18 Thread Joe Wang
in each LocPathIterator PredicatedNodeTest, and UnionPathIterator ? regards Mark *From:* core-libs-dev on behalf of Joe Wang *Sent:* Friday 15 May 2020 17:54 *To:* core-libs-dev@openjdk.java.net *Subject:* RFR [15] 8244342

RFR [15] 8244342: Compilation warnings about unexpected serialization related method signatures.

2020-05-15 Thread Joe Wang
Hi, Please review a fix for the compilation warnings. Thanks Roger for the detailed instructions! If you could verify the fix with the work-in-progress processor, that would be great too. Regular build and test passed. https://bugs.openjdk.java.net/browse/JDK-8244342

Re: RFR [15] 8245111: Update doc comments for improved processing by the Standard Doclet

2020-05-15 Thread Joe Wang
+1 Cheers, Joe On 5/15/2020 7:00 AM, Daniel Fuchs wrote: Hi Pavel, This looks good to me - but English is not my native language ;-) cheers, -- daniel On 15/05/2020 13:35, Pavel Rappo wrote: Hello, Please review this trivial change for https://bugs.openjdk.java.net/browse/JDK-8245111:

Re: RFR: JDK-8209774 - [TESTBUG]Refactor shell test javax/xml/jaxp/common/8035437/run.sh to java

2020-05-14 Thread Joe Wang
+1 Joe On 5/14/2020 2:14 AM, Daniel Fuchs wrote: Hi Fernando, That looks good to me. best regards, -- daniel On 12/05/2020 11:46, Fernando Guallini wrote: Thanks for your comments. Below you can find a new web rev version that includes a test description in a comment:

Re: [15] RFR: 8239383: Support for Unicode 13.0

2020-05-13 Thread Joe Wang
On 5/13/2020 3:24 PM, naoto.s...@oracle.com wrote: Thanks for the review, Joe! On 5/13/20 2:58 PM, Joe Wang wrote: +1 They are from the reference files, but it's interesting to note the ranges in Blocks.txt (that Character.java follows) for Tangut Supplement (18D00..18D8F) and CJK

Re: [15] RFR: 8239383: Support for Unicode 13.0

2020-05-13 Thread Joe Wang
+1 They are from the reference files, but it's interesting to note the ranges in Blocks.txt (that Character.java follows) for Tangut Supplement (18D00..18D8F) and CJK Unified Ideographs Extension G (3..3134F) are slightly different from those in the online pdf files

Re: RFR: JDK-8209774 - [TESTBUG]Refactor shell test javax/xml/jaxp/common/8035437/run.sh to java

2020-05-11 Thread Joe Wang
"/patches/java.xml/..”. To sum up, this test creates a temporary java.xml patch that needs to be ignored after compiling *DocumentImpl. *I am using —patch-module because it’s more flexible than @compile/module * * Hope I explained myself! This may be a question for Joe Wang but I'm cu

Re: RFR: JDK-8209774 - [TESTBUG]Refactor shell test javax/xml/jaxp/common/8035437/run.sh to java

2020-05-08 Thread Joe Wang
Hi Fernando, While you add the comment (about compile/run), you may want to update the original comment about the test as well as it mentioned "see run.sh" that is now gone. A minor comment on the header:  a space between the years would be good, that is "2014,2020," -> 2014, 2020, -Joe

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

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 exte

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

2020-05-07 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

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: 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.

Re: RFR[8183266] - [TESTBUG]Add test to cover XPathEvaluationResult.XPathResultType.getQNameType method

2020-04-30 Thread Joe Wang
Hi Fernando, Thanks for the update. Here's the updated webrev. The previous webrev was moved to webrev01. http://cr.openjdk.java.net/~joehw/jdk15/8183266/webrev02/ This change looks good to me. Best, Joe On 4/29/2020 9:38 AM, Joe Wang wrote: Hi Fernando, Thanks for adding coverage

<    1   2   3   4   5   6   7   8   9   >