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: JDK-8250611: Cannot display splash screen on Windows

2020-08-14 Thread alexander . matveev
Hi Andy, Looks good. Thanks, Alexander On 8/14/20 7:05 AM, Andy Herrick wrote: Please review revised webrev [3] that does not check for client jvm are (always use server) /Andy [3] - http://cr.openjdk.java.net/~herrick/8250611/webrev.02 On 8/13/2020 6:03 PM, Philip Race wrote: Do I

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

2020-08-14 Thread naoto . sato
Hello, Please review the fix for the following issue: https://bugs.openjdk.java.net/browse/JDK-8251499 The proposed changeset is located at: https://cr.openjdk.java.net/~naoto/8251499/webrev.00/ The current implementation of CompactNumberFormat assumes that there is always the number

Re: Fix for Javadoc errors in java.base

2020-08-14 Thread Vipin Sharma
Hi Sean, All 3 instances of @exception in DHPrivateKey are now replaced with @throws, added ProviderException similar to other constructor of DHPrivateKey. Updated patch: --- old/src/java.base/share/classes/com/sun/crypto/provider/DHPrivateKey.java 2020-08-14 23:55:40.921599987 +0530 +++

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

2020-08-14 Thread Joe Wang
Looks good, Lance. Nothing can't be solved with a cup of coffee ;-) Best, Joe On 8/14/20 10:43 AM, Lance Andersen wrote: Hi Joe, Thank you for the review. Changes had been made to the typos below (went back to far in IntelliJ fixing another issue :-( ) CSR has also been updated to fix

Re: RFR: JDK-8250611: Cannot display splash screen on Windows

2020-08-14 Thread Philip Race
+1 -phil On 8/14/20, 7:05 AM, Andy Herrick wrote: Please review revised webrev [3] that does not check for client jvm are (always use server) /Andy [3] - http://cr.openjdk.java.net/~herrick/8250611/webrev.02 On 8/13/2020 6:03 PM, Philip Race wrote: Do I understand correctly that you are

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

2020-08-14 Thread Lance Andersen
Hi Joe, Thank you for the review. Changes had been made to the typos below (went back to far in IntelliJ fixing another issue :-( ) CSR has also been updated to fix the typos (missed I guess due to lack of coffee this am ;-) ) http://cr.openjdk.java.net/~lancea/8251208/webrev.01/index.html

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: 8251160: Fix "no comment" warnings in java.logging

2020-08-14 Thread Mandy Chung
On 8/14/20 2:17 AM, Daniel Fuchs wrote: new webrev: http://cr.openjdk.java.net/~dfuchs/webrev_8251160/webrev.01/index.html +1 CSR updated as well: https://bugs.openjdk.java.net/browse/JDK-8251534 Reviewed. Mandy

Re: RFR: 8250803: pkgbuild failed with exit code 134

2020-08-14 Thread Andy Herrick
looks good. /Andy On 8/13/2020 4:13 PM, alexander.matv...@oracle.com wrote: Please review the jpackage fix for bug [1] at [2]. - Looks like issue was just in not enough time for test to execute. Increasing timeout by 1 min fixed issues. In fix it is increased by 50% just in case. Without

contributing to JDK-8233760 Result of BigDecimal.toString throws overflow exception on new BigDecimal(str)

2020-08-14 Thread Raffaello Giulietti
Hi, if nobody else is already fixing JDK-8233760 [1], I would like to start digging into it, as it still affects a recent changeset [2]. Greetings Raffaello [1] https://bugs.openjdk.java.net/browse/JDK-8233760 [2] http://hg.openjdk.java.net/jdk/jdk/rev/78705826c520

Re: RFR: 8245527: LDAP Cnannel Binding support for Java GSS/Kerberos

2020-08-14 Thread Aleks Efimov
Hi Alexey, The last version looks good to me. Best, Aleksei On 14/08/2020 15:42, Alexey Bakhtin wrote: Sorry, That’s my bad. I have reverted @systemProperty to @code. The wording is fixed. Webrev: http://cr.openjdk.java.net/~abakhtin/8245527/webrev.v17/ Regards Alexey On 14 Aug 2020, at

Re: RFR: 8245527: LDAP Cnannel Binding support for Java GSS/Kerberos

2020-08-14 Thread Daniel Fuchs
Hi Alexey, LGTM! Thanks, -- daniel On 14/08/2020 15:42, Alexey Bakhtin wrote: Sorry, That’s my bad. I have reverted @systemProperty to @code. The wording is fixed. Webrev: http://cr.openjdk.java.net/~abakhtin/8245527/webrev.v17/ Regards Alexey

Re: RFR: 8245527: LDAP Cnannel Binding support for Java GSS/Kerberos

2020-08-14 Thread Alexey Bakhtin
Sorry, That’s my bad. I have reverted @systemProperty to @code. The wording is fixed. Webrev: http://cr.openjdk.java.net/~abakhtin/8245527/webrev.v17/ Regards Alexey > On 14 Aug 2020, at 17:23, Sean Mullan wrote: > > Sorry you are right! Would be nice to have a more general @property for

Re: RFR: 8245527: LDAP Cnannel Binding support for Java GSS/Kerberos

2020-08-14 Thread Sean Mullan
Sorry you are right! Would be nice to have a more general @property for these types of properties and security properties, etc but that's a different change ... --Sean On 8/14/20 10:18 AM, Daniel Fuchs wrote: Hi Sean, Wait wait... Are they system properties really? Only system properties

Re: RFR: 8245527: LDAP Cnannel Binding support for Java GSS/Kerberos

2020-08-14 Thread Daniel Fuchs
Hi Sean, Wait wait... Are they system properties really? Only system properties should be documented with @systemProperty. I can't find the place were com.sun.jndi.ldap.connect.timeout or com.sun.jndi.ldap.read.timeout is read from System. I believe they are just plain environment properties

Re: RFR: JDK-8250611: Cannot display splash screen on Windows

2020-08-14 Thread Andy Herrick
Please review revised webrev [3] that does not check for client jvm are (always use server) /Andy [3] - http://cr.openjdk.java.net/~herrick/8250611/webrev.02 On 8/13/2020 6:03 PM, Philip Race wrote: Do I understand correctly that you are checking if the client VM is being specified ? I

Re: RFR: 8245527: LDAP Cnannel Binding support for Java GSS/Kerberos

2020-08-14 Thread Aleks Efimov
Hi Sean, Alexey, Adding @systemProperty doesn't look correct here since the properties mentioned in the module-info.java are not system properties, they're standard JNDI environment properties and setting them via system property with the same name won't have any effect, e.g. "java

Re: RFR: 8245527: LDAP Cnannel Binding support for Java GSS/Kerberos

2020-08-14 Thread Alexey Bakhtin
OK Updated for all properties in the module-info.java Webrev at: http://cr.openjdk.java.net/~abakhtin/8245527/webrev.v16/ Regards Alexey > On 14 Aug 2020, at 16:18, Sean Mullan wrote: > > On 8/14/20 8:41 AM, Alexey Bakhtin wrote: >> Hello Sean, >> Thank you for review. >> I’ve changed wording

Re: RFR: 8245527: LDAP Cnannel Binding support for Java GSS/Kerberos

2020-08-14 Thread Sean Mullan
On 8/14/20 8:41 AM, Alexey Bakhtin wrote: Hello Sean, Thank you for review. I’ve changed wording and replaced @code with @systemProperty (tested, it works for module-info.java) Thanks. Now that you know it works, I think you should change the other properties documented in that file to

Re: RFR: 8245527: LDAP Cnannel Binding support for Java GSS/Kerberos

2020-08-14 Thread Alexey Bakhtin
Hello Sean, Thank you for review. I’ve changed wording and replaced @code with @systemProperty (tested, it works for module-info.java) Updated webrev at: http://cr.openjdk.java.net/~abakhtin/8245527/webrev.v15/ Regards Alexey > On 14 Aug 2020, at 14:50, Sean Mullan wrote: > > On the

Re: RFR: 8245527: LDAP Cnannel Binding support for Java GSS/Kerberos

2020-08-14 Thread Sean Mullan
On the property wording, change "for LDAP connection" to "for an LDAP connection". Also, for the definition of the property, can you use the "@systemProperty" annotation instead of "@code"? Does that work inside the module-info.java file? I added my name as Reviewer. --Sean On 7/30/20

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

2020-08-14 Thread Lance Andersen
Hi all, Please review the fix to address javadoc warnings in java.sql and java.sql.rowsets The webrev can be found at: http://cr.openjdk.java.net/~lancea/8251208/webrev.00/ And the CSR at: https://bugs.openjdk.java.net/browse/JDK-8251834 Best Lance -- Lance Andersen|

Re: 8251160: Fix "no comment" warnings in java.logging

2020-08-14 Thread Daniel Fuchs
Hi Mandy, Thanks for looking at this! On 13/08/2020 20:48, Mandy Chung wrote: Level The javadoc for readResolve seems internal implementation details. Would it be sufficient to say: “Returns a {@code Level} instance with the same {@code name},  {@code value}, and {@code