Re: RFR: JDK11U JDK-8206120, JDK-8211398, JDK-8218915

Naoto Sato Thu, 21 Feb 2019 07:50:35 -0800

+1

Please follow the appropriate process to push the changeset.


Naoto

On 2/21/19 2:24 AM, Seán Coffey wrote:

Thanks. Looks good to me.

regards,
Sean.

On 21/02/2019 09:10, Deepak Kejriwal wrote:


Hi Sean,

The webrev I shared was not correct. I have corrected the webrev.02now. Please check now:-


http://cr.openjdk.java.net/~rpatil/JapaneseEra_changes_11u/webrev.02/

Regards,

Deepak

*From:*Seán Coffey
*Sent:* Thursday, February 21, 2019 2:11 PM

*To:* Deepak Kejriwal <deepak.kejri...@oracle.com>; Naoto Sato<naoto.s...@oracle.com>; core-libs-dev<core-libs-dev@openjdk.java.net>; jdk-updates-...@openjdk.java.net

*Subject:* Re: RFR: JDK11U JDK-8206120, JDK-8211398, JDK-8218915

Deepak,

this exception message in new test still needs correction:

      166                         "Character.isLetter(int) failed for codepoint 
"

      167                                 + Integer.toHexString(cp));

As an aside, there's probably no need for such specific exceptionmessages in a test case. It's error prone (but you've come this far now)


regards,
Sean.

On 21/02/2019 08:26, Deepak Kejriwal wrote:

    Hi Naoto,

    Corrected the exception message. Please find below updated version of 
webrev:-

    http://cr.openjdk.java.net/~rpatil/JapaneseEra_changes_11u/webrev.02/

    Thanks for review.

    Regards,

    Deepak

    -----Original Message-----

    From: Naoto Sato

    Sent: Wednesday, February 20, 2019 11:06 PM

    To: Deepak Kejriwal<deepak.kejri...@oracle.com>  <mailto:deepak.kejri...@oracle.com>; Sean 
Coffey<sean.cof...@oracle.com>  <mailto:sean.cof...@oracle.com>; 
core-libs-dev<core-libs-dev@openjdk.java.net>  
<mailto:core-libs-dev@openjdk.java.net>;jdk-updates-...@openjdk.java.net  
<mailto:jdk-updates-...@openjdk.java.net>

    Subject: Re: RFR: JDK11U JDK-8206120, JDK-8211398, JDK-8218915

    Hi Deepak,

    I see the following comment is not addressed yet:

      > - Line 163,198: Exception messages are incorrect. they are for 
isJavaIdentifierStart().

    Naoto

    On 2/20/19 8:53 AM, Deepak Kejriwal wrote:

        Thanks Naoto San and Sean for review. I have incorporate all the

        comments. Please find below updated version of webrev :-

        http://cr.openjdk.java.net/~rpatil/JapaneseEra_changes_11u/webrev.01/

        Regards,

        Deepak

        -----Original Message-----

        From: Naoto Sato

        Sent: Tuesday, February 19, 2019 10:23 PM

        To: Deepak Kejriwal<deepak.kejri...@oracle.com>  
<mailto:deepak.kejri...@oracle.com>; core-libs-dev

        <core-libs-dev@openjdk.java.net>  
<mailto:core-libs-dev@openjdk.java.net>;jdk-updates-...@openjdk.java.net  
<mailto:jdk-updates-...@openjdk.java.net>

        Subject: Re: RFR: JDK11U JDK-8206120, JDK-8211398, JDK-8218915

        Hi Deepak,

        Here are my comments to the webrev (other than what Sean pointed out):

        TestIsJavaIdentifierMethods.java

        - @summary: "testIsJavaLetter" -> "isJavaLetter", "testIsJavaLetterOrDigit" -> 
"isJavaLetterOrDigit".

        - Line 34: "newCodePoint" does not represent the era character, as "new"

        is subjective. It will become moot in the year 2020. How about 
"JAPANESE_ERA_CODEPOINT"?

        - Line 67,68,(...all the comments): Reflect the above change to the 
comments.

        - Line 103: "All Unicode chars (0x0000..0xFFFF)" does not sound correct.

        It may be "All Unicode code points in the BMP (0x0000..0xFFFF), and 
remove extra period at the end. This applies to other method descriptions.

        - Line 104: The test case returns "void", what does this "Expected 
results" mean?

        - Line 140-142,174-176: The condition statement in the document is 
different from JDK11's javadoc. In the API doc, it is (in case of int):

              isLetter(codePoint) returns true

              getType(codePoint) returns LETTER_NUMBER

              the referenced character is a currency symbol (such as '$')

              the referenced character is a connecting punctuation character 
(such as '_').

        - Line 163,198: Exception messages are incorrect. they are for 
isJavaIdentifierStart().

        Naoto

        On 2/19/19 6:15 AM, Deepak Kejriwal wrote:

            Correcting typo for release.

            From: Deepak Kejriwal<deepak.kejri...@oracle.com>  
<mailto:deepak.kejri...@oracle.com>

            Sent: Tuesday, February 19, 2019 7:42 PM

            To: 'core-libs-dev'<core-libs-dev@openjdk.java.net>  
<mailto:core-libs-dev@openjdk.java.net>;

            'jdk-updates-...@openjdk.java.net  
<mailto:jdk-updates-...@openjdk.java.net>'<jdk-updates-...@openjdk.java.net>  
<mailto:jdk-updates-...@openjdk.java.net>

            Subject: RFR: JDK11U JDK-8206120, JDK-8211398, JDK-8218915

            Hi All,

            Please review the backport of the following bug fixes to jdk11u-dev:

            HYPERLINK

            "https://bugs.openjdk.java.net/browse/JDK-8206120";  
<https://bugs.openjdk.java.net/browse/JDK-8206120>JDK-8206120 : Add

            test cases for lenient Japanese era parsing HYPERLINK

            "https://bugs.openjdk.java.net/browse/JDK-8211398";  
<https://bugs.openjdk.java.net/browse/JDK-8211398>JDK-8211398 :

            Square character support for the Japanese new era HYPERLINK

            "https://bugs.openjdk.java.net/browse/JDK-8218915";  
<https://bugs.openjdk.java.net/browse/JDK-8218915>JDK-8218915 :

            Change isJavaIdentifierStart and isJavaIdentifierPart to handle new

            code points

            Webrev:

            
http://cr.openjdk.java.net/~rpatil/JapaneseEra_changes_11u/webrev.00/

            These code changes are made possible thanks to specification change 
already pushed:

            http://hg.openjdk.java.net/jdk-updates/jdk11u/rev/c1e1669edace

            Regards,

            Deepak

Re: RFR: JDK11U JDK-8206120, JDK-8211398, JDK-8218915

Reply via email to