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

[Deepak Kejriwal]

Hi Sean,

 

The webrev I shared was not correct. I have corrected the webrev.02 now. 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.kejriwal at oracle.com>; Naoto Sato <naoto.sato at oracle.com>; core-libs-dev <core-libs-dev at openjdk.java.net>; jdk-updates-dev at 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 exception messages 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 HYPERLINK "mailto:deepak.kejriwal at oracle.com"<deepak.kejriwal at oracle.com>; Sean Coffey HYPERLINK "mailto:sean.coffey at oracle.com"<sean.coffey at oracle.com>; core-libs-dev HYPERLINK "mailto:core-libs-dev at openjdk.java.net"<core-libs-dev at openjdk.java.net>; HYPERLINK "mailto:jdk-updates-dev at openjdk.java.net"jdk-updates-dev at 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 HYPERLINK "mailto:deepak.kejriwal at oracle.com"<deepak.kejriwal at oracle.com>; core-libs-dev 
HYPERLINK "mailto:core-libs-dev at openjdk.java.net"<core-libs-dev at openjdk.java.net>; HYPERLINK "mailto:jdk-updates-dev at openjdk.java.net"jdk-updates-dev at 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 HYPERLINK "mailto:deepak.kejriwal at oracle.com"<deepak.kejriwal at oracle.com>
Sent: Tuesday, February 19, 2019 7:42 PM
To: 'core-libs-dev' HYPERLINK "mailto:core-libs-dev at openjdk.java.net"<core-libs-dev at openjdk.java.net>; 
'HYPERLINK "mailto:jdk-updates-dev at openjdk.java.net"jdk-updates-dev at openjdk.java.net' HYPERLINK "mailto:jdk-updates-dev at openjdk.java.net"<jdk-updates-dev at 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
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
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
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