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

feat(locale): add uzbek locale (uz_UZ_latin) #2686

Merged
merged 15 commits into from
Mar 4, 2024
Merged

Conversation

Mirazyzz
Copy link
Contributor

Added initial locale values for person in Uzbek language.

@Mirazyzz Mirazyzz requested a review from a team as a code owner February 25, 2024 22:13
@Mirazyzz Mirazyzz changed the title feat(locale): add uzbek locale feat(locale): add uzbek locale (uz-UZ) Feb 25, 2024
@ST-DDT ST-DDT added p: 1-normal Nothing urgent c: locale Permutes locale definitions m: person Something is referring to the person module labels Feb 25, 2024
@ST-DDT ST-DDT added this to the vAnytime milestone Feb 25, 2024
Copy link

codecov bot commented Feb 25, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.57%. Comparing base (c52ec8a) to head (31db724).

Additional details and impacted files
@@           Coverage Diff            @@
##             next    #2686    +/-   ##
========================================
  Coverage   99.57%   99.57%            
========================================
  Files        2846     2859    +13     
  Lines      248717   249642   +925     
  Branches     1007     1009     +2     
========================================
+ Hits       247651   248591   +940     
+ Misses       1037     1022    -15     
  Partials       29       29            
Files Coverage Δ
src/locale/index.ts 100.00% <100.00%> (ø)
src/locale/uz_UZ_latin.ts 100.00% <100.00%> (ø)
src/locales/index.ts 100.00% <100.00%> (ø)
src/locales/uz_UZ_latin/index.ts 100.00% <100.00%> (ø)
src/locales/uz_UZ_latin/metadata.ts 100.00% <100.00%> (ø)
src/locales/uz_UZ_latin/person/bio_parts.ts 100.00% <100.00%> (ø)
...rc/locales/uz_UZ_latin/person/female_first_name.ts 100.00% <100.00%> (ø)
src/locales/uz_UZ_latin/person/female_last_name.ts 100.00% <100.00%> (ø)
...les/uz_UZ_latin/person/female_last_name_pattern.ts 100.00% <100.00%> (ø)
src/locales/uz_UZ_latin/person/index.ts 100.00% <100.00%> (ø)
... and 5 more

... and 1 file with indirect coverage changes

Copy link
Member

@ST-DDT ST-DDT left a comment

Choose a reason for hiding this comment

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

IMO we should rename uz_UZ to uz.

@ST-DDT
Copy link
Member

ST-DDT commented Feb 25, 2024

Also please run pnpm run preflight

@Mirazyzz
Copy link
Contributor Author

Mirazyzz commented Feb 25, 2024

IMO we should rename uz_UZ to uz.

Uzbek language consist of latin and cyrillic alphabets. I thought we could include support for both in the future.
https://en.wikipedia.org/wiki/Uzbek_alphabet

@matthewmayer
Copy link
Contributor

IMO we should rename uz_UZ to uz.

For new locales i generally prefer lang_COUNTRY - as soon as you start adding addresses and phone numbers, they are typically specific for a certain country.

@ST-DDT
Copy link
Member

ST-DDT commented Feb 26, 2024

IMO we should rename uz_UZ to uz.

Uzbek language consist of latin and cyrillic alphabets. I thought we could include support for both in the future. https://en.wikipedia.org/wiki/Uzbek_alphabet

Ok, if there are two variants of the locale then we should make that visible in the name as well similar to https://github.com/faker-js/faker/tree/next/src/locales/sr_RS_latin

Or is there a different common disambiguator?

@Mirazyzz
Copy link
Contributor Author

Mirazyzz commented Feb 26, 2024

IMO we should rename uz_UZ to uz.

Uzbek language consist of latin and cyrillic alphabets. I thought we could include support for both in the future. https://en.wikipedia.org/wiki/Uzbek_alphabet

Ok, if there are two variants of the locale then we should make that visible in the name as well similar to https://github.com/faker-js/faker/tree/next/src/locales/sr_RS_latin

Or is there a different common disambiguator?

so would it be better to rename it to uz_latin? I don't think there is a common disambiguator that can be used like "uz_UZ" and "uz_CY" or something like that so we could probably use uz_latin and uz_cyrillic.

@Mirazyzz Mirazyzz requested a review from a team February 26, 2024 08:54
@ST-DDT
Copy link
Member

ST-DDT commented Feb 26, 2024

I meant more like uz_UZ_latin

Language_country_variant

@matthewmayer
Copy link
Contributor

uz_UZ_latin

Seems the most correct. Uzbek, as spoken in Uzbekistan, with Latin variant.

That would allow for say in future

uz_UZ_cyrillic
uz_AF_latin
uz_RU_cyrillic

src/locales/uz_UZ/metadata.ts Outdated Show resolved Hide resolved
src/locales/uz_UZ/person/bio_parts.ts Outdated Show resolved Hide resolved
@Mirazyzz
Copy link
Contributor Author

uz_UZ_latin

Seems the most correct. Uzbek, as spoken in Uzbekistan, with Latin variant.

That would allow for say in future

uz_UZ_cyrillic uz_AF_latin uz_RU_cyrillic

Just for clarity, the Uzbek language does not have dialectic variations such as American English, British English, or Australian English. The differences in Uzbek primarily lie within the alphabet utilized. Historically, we used the Cyrillic alphabet but transitioned to Latin over the past three decades, which is currently recognized as the official alphabet. Nevertheless, there remains a portion of the population, predominantly individuals over the age of 50-60, who only read in Cyrillic. The important point to note is that the grammar, words, and their meanings remain consistent regardless of the alphabet used.

Given this context, I think that it might be more reasonable to use codes "uz_latin" and "uz_cyrillic" rather than associating them with specific countries, since the language does not vary from one country to another. However, I'm open to different solutions just wanted to give more context.

@matthewmayer
Copy link
Contributor

Adding the country code, in addition to the alphabet variant is more about indicating that say faker.location.city() would return cities in Uzbekistan.

@Mirazyzz
Copy link
Contributor Author

@matthewmayer, @ST-DDT Is there any additional changes I have to make in order be able to merge?

@ST-DDT
Copy link
Member

ST-DDT commented Feb 28, 2024

There are two things required.

  • The team's decision on the locale id.
  • The review of the actual words (if you know somebody that is familiar with the language and help review the names and approve the PR, that would help us a lot). Otherwise it might take a few more days before we have the time to check the names/words.

@ST-DDT
Copy link
Member

ST-DDT commented Feb 29, 2024

Team Decision

Please use uz_UZ_latin to follow our localization guide.

We will extend the localization guide to include the representation <ISO-639-1>[_<ISO-3166-1>[_<variant>]] to better describe the intend behind our textual guideline.

We might change it in a future major version and then adjust all other locales as well.

@Mirazyzz
Copy link
Contributor Author

Mirazyzz commented Mar 1, 2024

Team Decision

Please use uz_UZ_latin to follow our localization guide.

We will extend the localization guide to include the representation <ISO-639-1>[_<ISO-3166-1>[_<variant>]] to better describe the intend behind our textual guideline.

We might change it in a future major version and then adjust all other locales as well.

Done.

@Mirazyzz Mirazyzz closed this Mar 1, 2024
@Mirazyzz Mirazyzz reopened this Mar 1, 2024
src/locales/uz_UZ_latin/metadata.ts Outdated Show resolved Hide resolved
@matthewmayer matthewmayer changed the title feat(locale): add uzbek locale (uz-UZ) feat(locale): add uzbek locale (uz_UZ_latin) Mar 2, 2024
ST-DDT
ST-DDT previously approved these changes Mar 4, 2024
Shinigami92
Shinigami92 previously approved these changes Mar 4, 2024
@Mirazyzz Mirazyzz dismissed stale reviews from Shinigami92 and ST-DDT via 31db724 March 4, 2024 14:12
@ST-DDT ST-DDT requested review from matthewmayer, ST-DDT and a team March 4, 2024 14:37
@ST-DDT ST-DDT merged commit f7471a2 into faker-js:next Mar 4, 2024
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: locale Permutes locale definitions m: person Something is referring to the person module p: 1-normal Nothing urgent
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants