On 13/05/2020 20:26, Severin Gehwolf wrote: > Hi Andrew, > > Thanks for looking at this. >
No problem. Thanks for taking it on. > On Wed, 2020-05-13 at 17:37 +0100, Andrew Hughes wrote: >> On 13/05/2020 15:59, Severin Gehwolf wrote: >>> Hi, >>> >>> Could somebody please review this fix for the failing >>> JapanEraNameCompatTest.java test (part of tier1)? The reason why it's >>> failing is that there are missing locale data for no and sr_Latn which >>> are exercised by the test. The original backport of JDK-8219781 didn't >>> include those as the changed sections in the JDK 11 patch didn't exist >>> in 8u. Those extra sections got introduced with JDK-8008577 or JDK- >>> 8145136. >> >> This also includes the JDK-8219781 changes for these locales. I think >> it's worth mentioning all three bug IDs in the summary text so they >> appear in a keyword search. > > OK, done. I was thinking more along the lines: Summary: Backports hr, lt, in, nl, no, sr_Latn & sv changes from 8008577, 8145136 & 8219781 Do we want this to appear as a backport bug of those issues? > >> Changes for these locales look ok to me. >> >>> This patch adds locale data for locales left out in the JDK-8219781 8u >>> backport, except for JavaTimeSupplementary_in.java. That file doesn't >>> exist at all in 8u, hence I've left that out. >> >> This doesn't make sense to me. The in locale is present in >> sun/text/resources, but the only one without JavaTimeSupplementary_*: > > Sorry for not being clear. I meant that there is no > JavaTimeSupplementary_in.java in 8u (prior this patch). Yeah, I got that. I just didn't understand why you didn't just add the file. snip... > > Sure. Done now: > http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8244843/02/webrev/ > ... > > How does the updated webrev above look to you? > Only issue with the patch itself is java.time.japanese.short.Eras is missing the new Reiwa era. I guess this is because 11u has JDK-8159943, which refactors these files so they share blocks where possible, before JDK-8219781. Thus, there is only one block to change in 11u, but it's a shared block which is then used from both java.time.japanese.short.Eras & java.time.japanese.long.Eras Looking at JDK-8219781 again, it is huge, but I don't think it actually changes much of the existing locales, just refactors them and adds some missing ones. I'm pondering whether a backport is worthwhile to avoid the duplication issue in future (without the LocaleData.java code changes). I guess it depends how likely it is we'll see more changes to locale data. Thanks, -- Andrew :) Senior Free Java Software Engineer Red Hat, Inc. (http://www.redhat.com) PGP Key: ed25519/0xCFDA0F9B35964222 (hkp://keys.gnupg.net) Fingerprint = 5132 579D D154 0ED2 3E04 C5A0 CFDA 0F9B 3596 4222