Masayoshi, Michael,
Many thanks for reviewing this fix and for all your comments.
Can I ask someone for a JDK8 commit sponsorship? The hg export is
located here:
http://cr.openjdk.java.net/~aefimov/8025051/8/8025051_jdk8.patch
Also, I want to take this opportunity and wish a Merry Christmas and
Happy New Year to all OpenJDK community members (at least members who
read this letter :) ).
-Aleksej
On 12/24/2013 10:39 AM, Masayoshi Okutsu wrote:
Looks good to me.
Masayoshi
On 12/23/2013 3:14 AM, Aleksej Efimov wrote:
Hi,
The new version of patch for TimeZone display names update is
available. Previous webrev contains incorrect naming for Acre
timezone generic name (ACT[] array) across all non-root locales. The
name was corrected and test TimeZoneNames_*.properties files were
modified accordingly.
The new webrev:
http://cr.openjdk.java.net/~aefimov/8025051/8/webrev.02/
<http://cr.openjdk.java.net/%7Eaefimov/8025051/8/webrev.02/>
-Aleksej
On 12/20/2013 03:39 PM, Aleksej Efimov wrote:
Masayoshi,
Thank you for the detailed review and your comments. I tried to
address all of them. The responses are below. The new webrev can be
found here: http://cr.openjdk.java.net/~aefimov/8025051/8/webrev.01/
<http://cr.openjdk.java.net/%7Eaefimov/8025051/8/webrev.01/>
Michael,
As Masayoshi mentioned, we have a list of inconsistent translations
in translation files. I suggest two approaches to resolve it: 1. Log
different bug with all problems in translation files. 2. Wait for
the next resource file translation update to address these problems.
-Aleksej
On 12/19/2013 12:04 PM, Masayoshi Okutsu wrote:
On 12/18/2013 6:43 PM, Aleksej Efimov wrote:
Hi,
Please help to review a fix [1] for 8025051 bug [2]. The following
fix includes:
Common to all modified files:
- All year ranges in the copyright header should be modified
accordingly.
Fixed.
- The translation of time zone generic names were added to all
locales.
I haven't fully reviewed translations, especially all \uxxxx
strings. But I noticed the following.
Common to all TimeZoneNames_*.java:
- The same generic abbreviation is used for the long name in MET. I
thought this was fixed in the root properties...
No, the "MET" is used in root properties both for generic long and
short names. I will update these names when new translation update
will be available.
- Some generic names don't match the style of their standard and/or
daylight time names.
The same as previous. This is a first large update for generic names
and latest translations. All inconsistency in translations will be
fixed during next translation update.
src/share/classes/sun/util/resources/de/TimeZoneNames_de.java:
- Some generic names use "Normalzeit". Is that OK?
It's not good. But the resolution is same as previous two.
src/share/classes/sun/util/resources/ja/TimeZoneNames_ja.java:
- "Chuuk Time" isn't translated.
I think it will be translated in next translations update.
- Time Zone names were updated according to the latest translations.
- Added tz names regression test
(test/sun/util/resources/TimeZone/TimeZoneNames) for a timezone
names across all locales. This test compares short/long
standard/daylight/generic names with translations stored in
.properties files.
test/sun/util/resources/TimeZone/TimeZoneNames/TimeZoneNamesTest.java:
- lines 33, 34: unused imports?
Removed
- line 75: Should it be detected as an error?
Agree, now the test will fail if there is some incorrect entries in
the test .properties files.
- line 108: IOException should be used as a cause. (OK to assume
"not found"?)
The IOException cause is added. Currently, the test will stop with
RuntimeException, because the test file is not found.
- lines 118 -121: Locale.getDefault() has to be replaced with
Locale.ROOT. Regression tests are run in different locales.
Thank you for catching this one. Fixed.
- Tests updates to address current changes (
GenericTimeZoneNamesTest.sh and Bug6317929.java).
The TODO comment in GenericTimeZoneNamesTest.sh should fully been
removed.
Removed
The following fix was tested with JPRT build and the 'jdk_util'
test set succeeded (new test is included in this test set).
Have you executed all date-time related regression tests (including
java.time and java.text)?
Yes, I have executed the following set of tests via JTREG:
test/sun/util/calendar test/java/util/Calendar
test/sun/util/resources/TimeZone test/sun/util/calendar
test/closed/java/util/TimeZone test/java/util/TimeZone
test/java/time test/java/util/Formatter
test/closed/java/util/Calendar
All tests gave "PASS" results.
Please, let me know if you think that some other tests should be
executed.
Thanks,
Masayoshi
[1] http://cr.openjdk.java.net/~aefimov/8025051/8/webrev.00/
[2] https://bugs.openjdk.java.net/browse/JDK-8025051
