That test edit looks fine Ramanand and will ensure that the test runs
remain green. I'll push this change for you now.
Regards,
Sean.
On 01/06/16 13:08, Ramanand Patil wrote:
Hi all,
As mentioned by point 4 in my first review thread, one test case is
still failing for which a separate JBS bug is created.(
https://bugs.openjdk.java.net/browse/JDK-8157792 ).
Below edit is needed for green tests and issue will be revisited via
JDK-8157792.
--- a/test/sun/util/calendar/zi/TestZoneInfo310.java
+++ b/test/sun/util/calendar/zi/TestZoneInfo310.java
@@ -164,6 +164,10 @@
}
for (String zid : zids_new) {
+ if (zid.equals("Asia/Oral") || zid.equals("Asia/Qyzylorda")) {
+ // JDK-8157792 tracking this issue
+ continue;
+ }
ZoneInfoOld zi = toZoneInfoOld(TimeZone.getTimeZone(zid));
ZoneInfoOld ziOLD = (ZoneInfoOld)ZoneInfoOld.getTimeZone(zid);
if (! zi.equalsTo(ziOLD)) {
Regards,
Ramanand.
*From:*Seán Coffey
*Sent:* Tuesday, May 31, 2016 3:05 PM
*To:* Masayoshi Okutsu; Ramanand Patil; i18n-...@openjdk.java.net;
core-libs-dev@openjdk.java.net
*Subject:* Re: RFR: 8151876: (tz) Support tzdata2016d
Masayoshi,
I still think the test adds value. At minimum it identifies timezones
which don't have a localised string in the JRE provider.
We need to start another discussion about how best to represent
timezone names for newly added timezones. At the moment, short and
long formats will be of "GMT±hh:mm" format. I suggest we use the IANA
timezone name for the long format name in future (e.g. "Asia/Tomsk"
instead of "GMT+06:00")
For the sake of getting this into the JDK code lines, I suggest we go
ahead with your suggestion to remove the test for now. I've also
reviewed this Ramanand. Your edits look fine (including test removal
for now)
Regards,
Sean.
On 31/05/2016 07:06, Masayoshi Okutsu wrote:
The CheckDisplayNames.java change is different from what I
suggested and doesn't make sense. But we no longer need the test.
I'd suggest CheckDisplayNames.java be removed. Otherwise, the fix
looks OK to me.
Masayoshi
On 5/31/2016 3:03 AM, Ramanand Patil wrote:
Hi Masayoshi and All,
Here is the updated Webrev:
http://cr.openjdk.java.net/~rpatil/8151876/webrev.01/
<http://cr.openjdk.java.net/%7Erpatil/8151876/webrev.01/>
As suggested by Masayoshi, I have kept the existing names
unchanged.
Also, this patch contains extra test case fixes for
1./java/util/TimeZone/CheckDisplayNames.java/
2.java/util/TimeZone/Bug8149452.java
The exclusion for the *newly* added TimeZones is added in
these test cases where the entries are not present in the
resources and the Short/Long display names fallback to the
GMT±hh:mm format.
Regards,
Ramanand.
*From:*Masayoshi Okutsu
*Sent:* Saturday, May 28, 2016 10:58 AM
*To:* Seán Coffey; Ramanand Patil; i18n-...@openjdk.java.net
<mailto:i18n-...@openjdk.java.net>;
core-libs-dev@openjdk.java.net
<mailto:core-libs-dev@openjdk.java.net>
*Subject:* Re: RFR: 8151876: (tz) Support tzdata2016d
CLDR locale data defines time zone names, like this (in en.xml):
<metazone type="Almaty">
<long>
<generic>Almaty Time</generic>
<standard>Almaty Standard
Time</standard>
<daylight>Almaty Summer
Time</daylight>
</long>
</metazone>
The CLDR converter tool tries to fill in the missing short
names from the legacy TimeZoneNames data. Removing existing
names causes some unexpected behavior. I think JDK-8157814 is
an example of the unexpected behavior. And the suggested fix
in JDK-8157814 looks to me like a workaround.
I still think the existing names should be kept unchanged for
this fix.
Thanks,
Masayoshi
On 5/28/2016 12:04 AM, Seán Coffey wrote:
I guess there's a low risk of timezone not being
identified if being parsed in through a formatter. Isn't
such an approach discouraged though ? short IDs were
already subject to change in tzdata releases. Ramanand
found one issue by removal of these resource strings (so
far) and that's captured in JDK-8157814
There's a decision to be made about how to use the
GMT±hh:mm format for new releases given IANA's new short
ID identifier mechanism. I think that could be discussed
as a separate issue. I'd like to see us follow a similar
approach to IANA and use GMT identifiers on new timezones
and perhaps even consider using the IANA long name format
also for the getDisplayName(..) calls that can be made.
e.g. "Asia/Almaty" instead of "Alma-Ata Time"
Regards,
Sean.
On 27/05/16 15:24, Masayoshi Okutsu wrote:
This change seems to beyond my proposal that the
"GMT±hh:mm" format is used for *new* zones with the
"±hh" format. But this change removes *existing* zones
which have changed to use the "±hh" format in tzdata.
Can this cause any compatibility issues?
And have we agreed to use the "GMT±hh:mm" format?
Thanks,
Masayoshi
On 5/27/2016 10:19 PM, Seán Coffey wrote:
Looks fine to me Ramanand. the recent 2016d changes
have introduced some boundary issues for JDK rule
parsing and those issues can be followed up in
separate issues like you say.
Regards,
Sean.
On 26/05/16 14:22, Ramanand Patil wrote:
HI all,
Please review the latest TZDATA integration
(tzdata2016d) to JDK9.
Bug: https://bugs.openjdk.java.net/browse/JDK-8151876
Webrev:
http://cr.openjdk.java.net/~rpatil/8151876/webrev.00/
<http://cr.openjdk.java.net/%7Erpatil/8151876/webrev.00/>
Patch Contains:
1. IANA tzdata2016d integration into JDK. [It
also includes tzdata2016b and tzdata2016c which was
not integrated].
2. "GMT[+ -]hh:mm" is used for formatting of the
modified or newly added TimeZones in tzdata2016d.
[This is done to accommodate the IANA's new system
where the zones use numeric time zone abbreviations
like "+04" instead of invented abbreviations like
"ASTT".]
3. Test case:
java/time/test/java/time/format/TestZoneTextPrinterParser.java
is updated to include the failures because of GMT[+
-]hh:mm format names.
4. Few other failing tests: For few other
failing tests, new linked bugs are created and will be
addressed in a separate patch.
Regards,
Ramanand.
