Hi Naoto, Can you review and approve the webrev? Thanks! https://cr.openjdk.java.net/~xliu/8234288/webrev.04/
Letu On 11/23/19, 6:09 PM, "naoto.s...@oracle.com" <naoto.s...@oracle.com> wrote: Looks good. Naoto On 11/22/19 9:55 PM, Yang, Letu wrote: > Hi Naoto, > > Added it in the new webrev > https://cr.openjdk.java.net/~xliu/8234288/webrev.03/ . Thanks! > > Letu > > > On Nov 22, 2019 4:40 PM, naoto.s...@oracle.com wrote: > Hi Letu, > > You might want to add lines for "Turkey" zone as well in LocaleData? > > Naoto > > On 11/22/19 4:16 PM, Yang, Letu wrote: >> Hi Naoto, >> >> Thank you for the advice! I've uploaded a new version: https://cr.openjdk.java.net/~xliu/8234288/webrev.02/webrev/ >> >> Letu >> >> On 11/21/19, 9:18 AM, "naoto.s...@oracle.com" <naoto.s...@oracle.com> wrote: >> >> Hi Letu, >> >> The change in the resource bundle file looks good. >> >> As to the regression test, I would avoid adding a separate file, instead >> add some variations for >> open/test/jdk/sun/text/resources/LocaleDataTest.java as I mentioned. Add >> some lines in "LocaleData" file, which contains the expected resources >> for the COMPAT locale provider. >> >> Naoto >> >> On 11/20/19 9:43 PM, Yang, Letu wrote: >> > Hi Naoto, >> > >> > Thank you for the suggestions! >> > >> > I've added a new webrev: https://cr.openjdk.java.net/~xliu/8234288/webrev.01/ >> > >> > Letu >> > >> > On 11/18/19, 9:09 AM, "naoto.s...@oracle.com" <naoto.s...@oracle.com> wrote: >> > >> > Hi Letu, >> > >> > Here are my comments to your changes: >> > >> > - You will need a regression test for this fix. Take a look at >> > test/jdk/sun/text/resources/LocaleDataTest.java, and add appropriate >> > test cases. >> > >> > - Fix comment should follow the OpenJDK changeset guideline [1] >> > >> > - As to the change itself, I would put "Turkey Summer Time"/"TRST" for >> > the 3rd and 4th array elements. Even though Turkey time do not observe >> > DST, names in those slots should reflect the DST (consistent to other names) >> > >> > - time zone id "Turkey" (line 1050) should also point to TRT array. >> > >> > Naoto >> > >> > [1] http://openjdk.java.net/guide/producingChangeset.html >> > >> > On 11/17/19 8:54 PM, Yang, Letu wrote: >> > > Hi Naoto, >> > > >> > > Thank you for the clarification! >> > > >> > > Xin from my team has filed a JBS and uploaded my webrev: >> > > https://bugs.openjdk.java.net/browse/JDK-8234288 >> > > https://cr.openjdk.java.net/~xliu/8234288/webrev.00/ >> > > >> > > Letu >> > > >> > > On 11/16/19, 6:44 AM, "naoto.s...@oracle.com" <naoto.s...@oracle.com> wrote: >> > > >> > > Letu, >> > > >> > > Please go ahead and fix the issue in English resource. As to the >> > > translation, Oracle l10n will translate it in appropriate locales. >> > > >> > > Naoto >> > > >> > > On 11/15/19 5:56 PM, Yang, Letu wrote: >> > > > Hi Naoto >> > > > >> > > > Thank you for the quick response! We will file a ticket later today. >> > > > >> > > > Shall we make an effort on fixing and translating the strings, or you >> > > > prefer to take care of it at Oracle? >> > > > >> > > > Letu >> > > > >> > > > On Nov 15, 2019 4:29 PM, naoto.s...@oracle.com wrote: >> > > > Hi Letu, >> > > > >> > > > Please file a JBS issue for this (component: core-libs, subcomponent: >> > > > java.util:i18n). >> > > > >> > > > Naoto >> > > > >> > > > On 11/15/19 3:19 PM, Yang, Letu wrote: >> > > >> Hi, >> > > >> >> > > >> We recently found an issue with the Time Zone name for “Europe/Istanbul” and "Asian/Istanbul". Since Turkey moved to their own Turkish Time (TRT) zone in 2016, although the tzdata had been updated, the Time Zone name string has not been updated yet: >> > > >> >> > > >> https://hg.openjdk.java.net/jdk/jdk/file/8e7f29b1ad4a/src/java.base/share/classes/sun/util/resources/TimeZoneNames.java#l836 >> > > >> >> > > >> It still returns "Eastern European Time" for the TimeZone.getDisplayName call, which has a summer time while Turkish Time does not. An entry for TRT need to be added to this file, and assign to both "Europe/Istanbul" and "Asian/Istanbul". This also need to be updated for other locales. I can create a JBS > issue for this, but I >> > > > am not sure whether we should fix this bug, or there is an existing >> > > > procedure for this kind of bug which requires language translation. >> > > >> >> > > >> Letu >> > > >> >> > > >> >> > > >> >> > > >> > > >> > >> > >> >>