Skip to content

CLDR-18253 Add en_JP #4634

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 2 commits into from
May 1, 2025
Merged

CLDR-18253 Add en_JP #4634

merged 2 commits into from
May 1, 2025

Conversation

AEApple
Copy link
Contributor

@AEApple AEApple commented Apr 23, 2025 •
edited
Loading

CLDR-18253

  • This PR completes the ticket.

ALLOW_MANY_COMMITS=true

macchiati
macchiati previously approved these changes Apr 23, 2025
View reviewed changes
@AEApple
Copy link
Contributor Author

AEApple commented Apr 25, 2025

Not sure why this is causing an error in German? That seem unexpected.

@srl295
Copy link
Member

srl295 commented Apr 25, 2025 

Error:  (TestExampleGenerator.java:2061)  Error: de	missing examples:	1

Done?	Without	Sample Attrs	URL	With	Sample Attrs	Section	Page	Starred Pattern
0.008438818565400843	235	japanese•0➔«Taika (645–650)»; japanese•100➔«Kaō (1169–1171)»; japanese•101➔«Shōan (1171–1175)»; japanese•102➔«Angen (1175–1177)»; japanese•103➔«Jishō (1177–1181)»	https://st.unicode.org/cldr-apps/v#/de//27027cf9e26792f0	2	japanese•236➔«R»	Date & Time	Japanese	//ldml/dates/calendars/calendar[@type="*"]/eras/eraNarrow/era[@type="*"]

@srl295
Copy link
Member

srl295 commented Apr 25, 2025 •
edited
Loading

  • The test (which has several problems, including clarity) is actually testing english coverage levels, despite the locale ID

  • English now includes Japan - en-JP

  • Therefore, you hit this rule <coverageLevel value="moderate" inTerritory="JP" match="dates/calendars/calendar[@type='japanese']/eras/eraNarrow/era[@type='%japaneseEras']"/>

  • Therefore, the path is subject to requiring an example

  • Really, no reason these eras shouldn't have an example (why don't all of them?) so it's probably a latent bug.

  • Separate question is whether the eras are now moderate coverage for all english sublocales. (this PR as is will require all english sublocales to have japanese eras)

in short working as designed though very confusing

@srl295
Copy link
Member

srl295 commented Apr 25, 2025

The reason the eras aren't in the examples is due to CLDR-17851 which should have been a logKnownIssue

@srl295
Copy link
Member

srl295 commented Apr 25, 2025 •
edited
Loading

passes when #4642 is applied.

AEApple added a commit to AEApple/cldr that referenced this pull request Apr 27, 2025
@AEApple
CLDR-18253 Add en_JP
834794c 
See unicode-org#4634
@jira-pull-request-webhook
Copy link

Hooray! The files in the branch are the same across the force-push. 😃

~ Your Friendly Jira-GitHub PR Checker Bot

@AEApple AEApple changed the title Test sideways inheritance fix CLDR-18253 Add en_JP Apr 27, 2025
AEApple added a commit to AEApple/cldr that referenced this pull request Apr 27, 2025
@AEApple
CLDR-18253 Add en_JP
b6aed84 
See unicode-org#4634
@jira-pull-request-webhook
Copy link

Hooray! The files in the branch are the same across the force-push. 😃

~ Your Friendly Jira-GitHub PR Checker Bot

@AEApple
Copy link
Contributor Author

AEApple commented Apr 27, 2025

This one should pass once #4642 is approved so it can be merged.

@AEApple AEApple marked this pull request as ready for review April 27, 2025 21:50
pedberg-icu
pedberg-icu reviewed Apr 28, 2025
View reviewed changes
common/main/en_JP.xml Outdated
</dateFormatLength>
<dateFormatLength type="short">
<dateFormat>
<pattern>y/M/d</pattern>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apple's en_JP locale (which I can see from the open-source Darwin project) has "y/MM/dd" for this

pedberg-icu
pedberg-icu reviewed Apr 28, 2025
View reviewed changes
common/main/en_JP.xml Outdated
<dateFormatLength type="short">
<dateFormat>
<pattern>y/M/d</pattern>
<datetimeSkeleton>yyMd</datetimeSkeleton>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should match the pattern, so if the pattern has one y, this should too.

pedberg-icu
pedberg-icu reviewed Apr 28, 2025
View reviewed changes
common/main/en_JP.xml Outdated
<timeFormats>
<timeFormatLength type="full">
<timeFormat>
<pattern>HH:mm:ss zzzz</pattern>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apple's en_JP locale has a single H for these time patterns.

pedberg-icu
pedberg-icu reviewed Apr 28, 2025
View reviewed changes
common/main/en_JP.xml Outdated
<dateFormats>
<dateFormatLength type="full">
<dateFormat>
<pattern>EEEE, MMMM d, y G</pattern>
Copy link
Contributor

@pedberg-icu pedberg-icu Apr 28, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apple's en_JP locale has "G y MMMM d, EEEE" here, and similar ordering for the long & medium formats.

pedberg-icu
pedberg-icu reviewed Apr 28, 2025
View reviewed changes
common/main/en_JP.xml Outdated
</dateFormatLength>
<dateFormatLength type="short">
<dateFormat>
<pattern>y/M/d GGGGG</pattern>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apple's en_JP locale has "GGGGG y/MM/dd" here (consistent with the MM/dd for gregorian)

macchiati
macchiati reviewed Apr 28, 2025
View reviewed changes
Copy link
Member

@macchiati macchiati left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good except for items Peter noted. I have no strong feelings either way, but we might as well not deviate from what Apple is doing.

@AEApple
Copy link
Contributor Author

AEApple commented Apr 28, 2025

I will update it based on Peter's feedback and then tag you all for re-review.

pedberg-icu
pedberg-icu reviewed Apr 29, 2025
View reviewed changes
common/main/en_JP.xml Outdated
<greatestDifference id="y">y – y G</greatestDifference>
</intervalFormatItem>
<intervalFormatItem id="GyM">
<greatestDifference id="G">MM/y GGGGG – MM/y GGGGG</greatestDifference>
Copy link
Contributor

@pedberg-icu pedberg-icu Apr 29, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pattern should be GGGGG y/MM – GGGGG y/MM

pedberg-icu
pedberg-icu reviewed Apr 29, 2025
View reviewed changes
common/main/en_JP.xml Outdated
</intervalFormatItem>
<intervalFormatItem id="GyM">
<greatestDifference id="G">MM/y GGGGG – MM/y GGGGG</greatestDifference>
<greatestDifference id="M">MM/y – MM/y GGGGG</greatestDifference>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pattern for this and next should be GGGGG y/MM – y/MM

pedberg-icu
pedberg-icu reviewed Apr 29, 2025
View reviewed changes
common/main/en_JP.xml Outdated
<greatestDifference id="y">MM/y – MM/y GGGGG</greatestDifference>
</intervalFormatItem>
<intervalFormatItem id="GyMd">
<greatestDifference id="d">y/MM/dd – y/MM/dd GGGGG</greatestDifference>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pattern should have GGGGG first: GGGGG y/MM/dd – y/MM/dd

pedberg-icu
pedberg-icu reviewed Apr 29, 2025
View reviewed changes
common/main/en_JP.xml Outdated
</intervalFormatItem>
<intervalFormatItem id="GyMd">
<greatestDifference id="d">y/MM/dd – y/MM/dd GGGGG</greatestDifference>
<greatestDifference id="G">y/MM/dd GGGGG – y/MM/dd GGGGG</greatestDifference>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pattern should have GGGGG first: GGGGG y/MM/dd – GGGGG y/MM/dd

pedberg-icu
pedberg-icu reviewed Apr 29, 2025
View reviewed changes
common/main/en_JP.xml Outdated
<greatestDifference id="y">y/MM/dd – y/MM/dd GGGGG</greatestDifference>
</intervalFormatItem>
<intervalFormatItem id="GyMEd">
<greatestDifference id="d">E, y/MM/dd – E, y/MM/dd GGGGG</greatestDifference>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As above, for this and the rest of the numeric date formats below, ordering should be "GGGGG y/MM/dd E" or a subset thereof.

@pedberg-icu
Copy link
Contributor

In general, the main issue I see is that across all of the calendars, the short/numeric date formats (for single-M skeletons) should consistently have the pattern "GGGGG y/MM/dd E" or the appropriate subset of that. Currently there is some variation in order.

@AEApple AEApple requested a review from pedberg-icu April 29, 2025 13:28
@AEApple
Copy link
Contributor Author

AEApple commented Apr 29, 2025

@pedberg-icu - Is it better to have the Japanese eras in en.xml and add an exception for them, or only have them in en_JP.xml? Root doesn't have the same errors, but root Japanese eras are very long since they always include the Gregorian years.

AEApple added a commit to AEApple/cldr that referenced this pull request Apr 29, 2025
@AEApple
CLDR-18253 Add en_JP
926a748 
See unicode-org#4634
@jira-pull-request-webhook
Copy link

Hooray! The files in the branch are the same across the force-push. 😃

~ Your Friendly Jira-GitHub PR Checker Bot

@AEApple
Copy link
Contributor Author

AEApple commented Apr 29, 2025

Okay, so this is still just blocked by the example generator issue. Let me know if there are any other tweaks to the data, otherwise I would like to submit this after it is unblocked.

@macchiati
Copy link
Member

I think it's been merged, so we need to re-kick-off the build.

@macchiati
Copy link
Member

Just retriggered.

macchiati
macchiati previously approved these changes Apr 29, 2025
View reviewed changes
srl295
srl295 previously approved these changes Apr 30, 2025
View reviewed changes
@AEApple AEApple dismissed stale reviews from srl295 and macchiati via d144fb5 April 30, 2025 23:22
AEApple added a commit to AEApple/cldr that referenced this pull request Apr 30, 2025
@AEApple
CLDR-18253 Add en_JP
37eee58 
See unicode-org#4634
@jira-pull-request-webhook
Copy link

Hooray! The files in the branch are the same across the force-push. 😃

~ Your Friendly Jira-GitHub PR Checker Bot

@AEApple AEApple requested review from srl295 and macchiati April 30, 2025 23:57
@AEApple
Copy link
Contributor Author

AEApple commented May 1, 2025 •
edited
Loading

@macchiati - How do I troubleshoot an unspecified English inheritance error, actually let me try something.

@jira-pull-request-webhook
Copy link

Hooray! The files in the branch are the same across the force-push. 😃

~ Your Friendly Jira-GitHub PR Checker Bot

@AEApple AEApple merged commit 105fd62 into unicode-org:main May 1, 2025
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@macchiati macchiati macchiati approved these changes

@conradarcturus conradarcturus Awaiting requested review from conradarcturus

@DraganBesevic DraganBesevic Awaiting requested review from DraganBesevic

@pedberg-icu pedberg-icu Awaiting requested review from pedberg-icu

@srl295 srl295 Awaiting requested review from srl295

Assignees

@AEApple AEApple

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@AEApple @srl295 @macchiati @pedberg-icu