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; 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.



Reply via email to