Roll pub packages manually#145170
Conversation
github-actions
bot
added
tool
| expect(localizations.selectedRowCountTitle(10000), 'បានជ្រើសរើសធាតុ 10.000'); | ||
| expect(localizations.selectedRowCountTitle(123456789), 'បានជ្រើសរើសធាតុ 123.456.789'); | ||
| expect(localizations.selectedRowCountTitle(10000), 'បានជ្រើសរើសធាតុ 10,000'); | ||
| expect(localizations.selectedRowCountTitle(123456789), 'បានជ្រើសរើសធាតុ 123,456,789'); |
There was a problem hiding this comment.
@goderbauer and I did some research:
- Wikipedia article indicates Khmer uses commas as the thousands separator: https://en.wikipedia.org/wiki/Khmer_numerals#Numbers_from_100_to_10,000,000
- Google Translate uses commas as the thousands separator: https://translate.google.com/?sl=en&tl=km&text=I%20have%201%2C000%20apples&op=translate
- Gemini agrees it should be a comma:
The Khmer language uses a comma (,) as its thousands separator. However, you might also see spaces used as a thousands separator, especially in less formal contexts.
EDIT: Copying this comment: #145167 (comment)
Just FYI: The authority here is the Unicode CLDR data, which agrees on the grouping separator being a comma.
There was a problem hiding this comment.
(Drive-by reviewing, feel free to ignore) ICU says:
https://www.localeplanet.com/icu/km-KH/index.html
Numbers
- Grouping separator:
.- Decimal separator:
,
Sauce:
There was a problem hiding this comment.
Sometimes software is too hard
There was a problem hiding this comment.
Looks like this change is correct, per: #145167 (comment)
There was a problem hiding this comment.
Hah, and people complain about timezones :P What an amazing "Winning" column in that link :P
There was a problem hiding this comment.
This is where the separators flipped:
https://unicode.org/cldr/charts/44/delta/km.html
(Looks like ICU4c hasn't regenerated data from the latest CLDR yet?)
loic-sharma
added
the
autosubmit
eliasyishak
left a comment
eliasyishak
left a comment
There was a problem hiding this comment.
I need to make an update to analytics for 5.8.7, can we wait until that publishes in a few minutes?
auto-submit
bot
removed
the
autosubmit
|
auto label is removed for flutter/flutter/145170, due to This PR has not met approval requirements for merging. Changes were requested by {eliasyishak}, please make the needed changes and resubmit this PR.
|
|
@eliasyishak No problem. Let me know when you're good to go! |
|
For context, Elias and I believe that if we merge this as is, the version of unified_analytics here will make dart-lang/tools#252 even worse, so we're going to try to roll this forward two hotfixes. |
eliasyishak
added
the
autosubmit
5 participants
The automated roll failed as a test needs to be updated: #145167
Fixes: #139861
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.