Skip to content
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.

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

CLDR-17839 Fix units in he #3910

Merged
merged 1 commit into from
Aug 6, 2024
Merged

CLDR-17839 Fix units in he #3910

merged 1 commit into from
Aug 6, 2024

Conversation

AEApple
Copy link
Contributor

@AEApple AEApple commented Jul 26, 2024

CLDR-17839

Update he narrow units to inherit from short

  • This PR completes the ticket.

ALLOW_MANY_COMMITS=true

@AEApple AEApple self-assigned this Jul 26, 2024
@AEApple
Copy link
Contributor Author

AEApple commented Jul 26, 2024

@macchiati - Does short inherit from narrow instead of the other way around? I only changed 'narrow' to inherit, so I don't understand why we would require a value for two under narrow but not short?

@AEApple
Copy link
Contributor Author

AEApple commented Jul 26, 2024

nm, maybe it's just due to a typo I had.

@macchiati
Copy link
Member

You can get rid of some of the errors by rebasing onto main. I can help with the others on Monday.

@macchiati
Copy link
Member

When you get back, can you rebase this on main? We can then take a look at why the other errors are happening.

@AEApple
Copy link
Contributor Author

AEApple commented Aug 1, 2024

interestingly it only gave me the option to refresh with a merge commit and not rebase

@AEApple
Copy link
Contributor Author

AEApple commented Aug 1, 2024

@macchiati - I think you have to fix the plurals issue first and then I should make my change on top of it. Right now I'm seeing errors due to items such as: https://st.unicode.org/cldr-apps/v#/he/Graphics/665721dc2c8221b3 which is unexpected since two is only used for two per the plural charts: https://www.unicode.org/cldr/charts/45/supplemental/language_plural_rules.html#he

@macchiati
Copy link
Member

I'm not sure what is going on here. (Sorry I missed your message of 4 days ago.) I'll have to investigate tomorrow.

Check failure on line 12161 in common/main/he.xml
inheritance marker not allowed
Error: Inheritance marker ↑↑↑ not allowed in a data value.

Very odd. The check appears to be malfunctioning, because ↑↑↑ is allowed everywhere inheritance is.

Check failure on line 12487 in common/main/he.xml
missing placeholders
Error: Need at least 1 placeholder(s), but only have 0. Placeholders are: {{0}={NUMBER_OF_UNITS}, e.g. “3”}; see missing placeholders.

It is very strange because in many other cases "‎↑↑↑" works fine.

AEApple added a commit that referenced this pull request Aug 5, 2024
@AEApple AEApple force-pushed the AEApple-CLDR-17839 branch from da0b787 to b85039d Compare August 5, 2024 07:24
@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 Aug 5, 2024

Okay, well I went through and updated everything which seemed like it matched. There were a few things I was less sure about (e.g. durations). I sometimes had a more abbreviated value inherit where it seemed safer.

macchiati
macchiati previously approved these changes Aug 5, 2024
@macchiati
Copy link
Member

Whew, a lot of work there! Approved

srl295
srl295 previously approved these changes Aug 5, 2024
@AEApple
Copy link
Contributor Author

AEApple commented Aug 5, 2024

I'm having people review in staging so that I can make any change if needed. I will merge before Dragan starts the alpha1 integration though.

Copy link
Contributor Author

@AEApple AEApple left a comment

Choose a reason for hiding this comment

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

CLDR-17839 Fix two in durations

Copy link
Contributor Author

@AEApple AEApple left a comment

Choose a reason for hiding this comment

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

CLDR-17839 Fix two in durations

@AEApple AEApple dismissed stale reviews from srl295 and macchiati via abb5756 August 6, 2024 19:09
CLDR-17839 Update he narrow units to inherit from short

See #3910
@AEApple AEApple force-pushed the AEApple-CLDR-17839 branch from 417063c to 03ff4e5 Compare August 6, 2024 19:11
@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 macchiati and srl295 August 6, 2024 19:11
@AEApple AEApple changed the title CLDR-17839 Update he narrow units to inherit from short CLDR-17839 Fix units in he Aug 6, 2024
@macchiati macchiati merged commit 52afa46 into main Aug 6, 2024
13 checks passed
@AEApple AEApple deleted the AEApple-CLDR-17839 branch August 6, 2024 22:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants