RFR 8232161: Align some one-way conversion in MS950 charset with Windows
Hello. Could you review the fix ? Bug: https://bugs.openjdk.java.net/browse/JDK-8232161 Change: https://cr.openjdk.java.net/~itakiguchi/8232161/webrev.01/ CSR 8233385 [1] was approved. [1] https://bugs.openjdk.java.net/browse/JDK-8233385 Thanks, Ichiroh Takiguchi IBM Japan, Ltd.
Hi Takiguchi-san, A few comments: - I'd recommend sorting the entries in MS950.nr and test data in TestMS950.java for readability. - Add some comment about the objective in the test. It'd be hard for engineers who have no previous knowledge to these bytes. Naoto On 3/2/20 9:33 AM, Ichiroh Takiguchi wrote:
Hello.
Could you review the fix ?
Bug: https://bugs.openjdk.java.net/browse/JDK-8232161 Change: https://cr.openjdk.java.net/~itakiguchi/8232161/webrev.01/
CSR 8233385 [1] was approved.
[1] https://bugs.openjdk.java.net/browse/JDK-8233385
Thanks, Ichiroh Takiguchi IBM Japan, Ltd.
Hello Naoto. I appreciate your comments. I applied following changes: * MS950.nr and TestMS950.java data were sorted by Unicode order * Added some comments into TestMS950.java * Change comment on MS950.map Could you review the fix ? Bug: https://bugs.openjdk.java.net/browse/JDK-8232161 Change: https://cr.openjdk.java.net/~itakiguchi/8232161/webrev.02/ Thanks, Ichiroh Takiguchi On 2020-03-03 10:31, naoto.sato@oracle.com wrote:
Hi Takiguchi-san,
A few comments:
- I'd recommend sorting the entries in MS950.nr and test data in TestMS950.java for readability.
- Add some comment about the objective in the test. It'd be hard for engineers who have no previous knowledge to these bytes.
Naoto
On 3/2/20 9:33 AM, Ichiroh Takiguchi wrote:
Hello.
Could you review the fix ?
Bug: https://bugs.openjdk.java.net/browse/JDK-8232161 Change: https://cr.openjdk.java.net/~itakiguchi/8232161/webrev.01/
CSR 8233385 [1] was approved.
[1] https://bugs.openjdk.java.net/browse/JDK-8233385
Thanks, Ichiroh Takiguchi IBM Japan, Ltd.
On 3/4/20 9:18 AM, Ichiroh Takiguchi wrote:
Hello Naoto.
I appreciate your comments.
I applied following changes: * MS950.nr and TestMS950.java data were sorted by Unicode order * Added some comments into TestMS950.java * Change comment on MS950.map
Could you review the fix ?
Bug: https://bugs.openjdk.java.net/browse/JDK-8232161 Change: https://cr.openjdk.java.net/~itakiguchi/8232161/webrev.02/
I'd expect the sort order to be aligned with other *.nr files, i.e., sorted by the source byte sequence. Same for the test case (TestMS950.java) and the comment in MS950.map. As to the test comment, how about adding something below to @summary line? "Those test data confirm the preferred b2c irreversible mappings defined in MS950.nr file." Naoto
Thanks, Ichiroh Takiguchi
On 2020-03-03 10:31, naoto.sato@oracle.com wrote:
Hi Takiguchi-san,
A few comments:
- I'd recommend sorting the entries in MS950.nr and test data in TestMS950.java for readability.
- Add some comment about the objective in the test. It'd be hard for engineers who have no previous knowledge to these bytes.
Naoto
On 3/2/20 9:33 AM, Ichiroh Takiguchi wrote:
Hello.
Could you review the fix ?
Bug: https://bugs.openjdk.java.net/browse/JDK-8232161 Change: https://cr.openjdk.java.net/~itakiguchi/8232161/webrev.01/
CSR 8233385 [1] was approved.
[1] https://bugs.openjdk.java.net/browse/JDK-8233385
Thanks, Ichiroh Takiguchi IBM Japan, Ltd.
Hello Naoto. I appreciate your comments. I modified TestMS950.java testcase. I think it's easy to read. Could you review the fix again ? Bug: https://bugs.openjdk.java.net/browse/JDK-8232161 Change: https://cr.openjdk.java.net/~itakiguchi/8232161/webrev.03/ Thanks, Ichiroh Takiguchi On 2020-03-05 04:31, naoto.sato@oracle.com wrote:
On 3/4/20 9:18 AM, Ichiroh Takiguchi wrote:
Hello Naoto.
I appreciate your comments.
I applied following changes: * MS950.nr and TestMS950.java data were sorted by Unicode order * Added some comments into TestMS950.java * Change comment on MS950.map
Could you review the fix ?
Bug: https://bugs.openjdk.java.net/browse/JDK-8232161 Change: https://cr.openjdk.java.net/~itakiguchi/8232161/webrev.02/
I'd expect the sort order to be aligned with other *.nr files, i.e., sorted by the source byte sequence. Same for the test case (TestMS950.java) and the comment in MS950.map.
As to the test comment, how about adding something below to @summary line?
"Those test data confirm the preferred b2c irreversible mappings defined in MS950.nr file."
Naoto
Thanks, Ichiroh Takiguchi
On 2020-03-03 10:31, naoto.sato@oracle.com wrote:
Hi Takiguchi-san,
A few comments:
- I'd recommend sorting the entries in MS950.nr and test data in TestMS950.java for readability.
- Add some comment about the objective in the test. It'd be hard for engineers who have no previous knowledge to these bytes.
Naoto
On 3/2/20 9:33 AM, Ichiroh Takiguchi wrote:
Hello.
Could you review the fix ?
Bug: https://bugs.openjdk.java.net/browse/JDK-8232161 Change: https://cr.openjdk.java.net/~itakiguchi/8232161/webrev.01/
CSR 8233385 [1] was approved.
[1] https://bugs.openjdk.java.net/browse/JDK-8233385
Thanks, Ichiroh Takiguchi IBM Japan, Ltd.
Looks good to me. Thanks for the update. Naoto On 3/9/20 10:37 AM, Ichiroh Takiguchi wrote:
Hello Naoto.
I appreciate your comments. I modified TestMS950.java testcase. I think it's easy to read.
Could you review the fix again ?
Bug: https://bugs.openjdk.java.net/browse/JDK-8232161 Change: https://cr.openjdk.java.net/~itakiguchi/8232161/webrev.03/
Thanks, Ichiroh Takiguchi
On 2020-03-05 04:31, naoto.sato@oracle.com wrote:
On 3/4/20 9:18 AM, Ichiroh Takiguchi wrote:
Hello Naoto.
I appreciate your comments.
I applied following changes: * MS950.nr and TestMS950.java data were sorted by Unicode order * Added some comments into TestMS950.java * Change comment on MS950.map
Could you review the fix ?
Bug: https://bugs.openjdk.java.net/browse/JDK-8232161 Change: https://cr.openjdk.java.net/~itakiguchi/8232161/webrev.02/
I'd expect the sort order to be aligned with other *.nr files, i.e., sorted by the source byte sequence. Same for the test case (TestMS950.java) and the comment in MS950.map.
As to the test comment, how about adding something below to @summary line?
"Those test data confirm the preferred b2c irreversible mappings defined in MS950.nr file."
Naoto
Thanks, Ichiroh Takiguchi
On 2020-03-03 10:31, naoto.sato@oracle.com wrote:
Hi Takiguchi-san,
A few comments:
- I'd recommend sorting the entries in MS950.nr and test data in TestMS950.java for readability.
- Add some comment about the objective in the test. It'd be hard for engineers who have no previous knowledge to these bytes.
Naoto
On 3/2/20 9:33 AM, Ichiroh Takiguchi wrote:
Hello.
Could you review the fix ?
Bug: https://bugs.openjdk.java.net/browse/JDK-8232161 Change: https://cr.openjdk.java.net/~itakiguchi/8232161/webrev.01/
CSR 8233385 [1] was approved.
[1] https://bugs.openjdk.java.net/browse/JDK-8233385
Thanks, Ichiroh Takiguchi IBM Japan, Ltd.
participants (2)
-
Ichiroh Takiguchi
-
naoto.sato＠oracle.com