Looks good, assuming the change between 03 and 04 is to fix "no new line..."
Naoto
On 12/4/19 11:38 AM, Yang, Letu wrote:
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
>> > > >>
>> > > >>
>> > > >>
>> > >
>> > >
>> >
>> >
>>
>>
