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.

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

feat(locale): add location for uz_UZ_latin #2775

Merged
merged 8 commits into from
Apr 2, 2024
Merged

feat(locale): add location for uz_UZ_latin #2775

merged 8 commits into from
Apr 2, 2024

Conversation

Mirazyzz
Copy link
Contributor

No description provided.

@Mirazyzz Mirazyzz requested a review from a team as a code owner March 27, 2024 11:41
@netlify Netlify
Copy link

netlify bot commented Mar 27, 2024 •
edited

✅ Deploy Preview for fakerjs ready!

Name Link
🔨 Latest commit 9f23638
🔍 Latest deploy log https://app.netlify.com/sites/fakerjs/deploys/660b7d73b7d61700089df90f
😎 Deploy Preview https://deploy-preview-2775.fakerjs.dev
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@codecov Codecov
Copy link

codecov bot commented Mar 27, 2024 •
edited

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.94%. Comparing base (2e97b52) to head (9f23638).

Additional details and impacted files 
@@            Coverage Diff             @@
##             next    #2775      +/-   ##
==========================================
- Coverage   99.94%   99.94%   -0.01%     
==========================================
  Files        2960     2973      +13     
  Lines      211726   212530     +804     
  Branches      602      954     +352     
==========================================
+ Hits       211606   212403     +797     
- Misses        116      123       +7     
  Partials        4        4
Files Coverage Δ
src/locales/uz_UZ_latin/index.ts 100.00% <100.00%> (ø)
...rc/locales/uz_UZ_latin/location/building_number.ts 100.00% <100.00%> (ø)
src/locales/uz_UZ_latin/location/city_name.ts 100.00% <100.00%> (ø)
src/locales/uz_UZ_latin/location/city_pattern.ts 100.00% <100.00%> (ø)
src/locales/uz_UZ_latin/location/country.ts 100.00% <100.00%> (ø)
src/locales/uz_UZ_latin/location/direction.ts 100.00% <100.00%> (ø)
src/locales/uz_UZ_latin/location/index.ts 100.00% <100.00%> (ø)
src/locales/uz_UZ_latin/location/postcode.ts 100.00% <100.00%> (ø)
.../locales/uz_UZ_latin/location/secondary_address.ts 100.00% <100.00%> (ø)
src/locales/uz_UZ_latin/location/state.ts 100.00% <100.00%> (ø)
... and 4 more

... and 2 files with indirect coverage changes

ST-DDT
ST-DDT reviewed Mar 27, 2024
View reviewed 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.

Please sort the files alphabetically.

src/locales/uz_UZ_latin/location/street_pattern.ts Outdated Show resolved Hide resolved
@Mirazyzz
Copy link
Contributor Author

Please sort the files alphabetically.

Do you want the street names to be sorted alphabetically? They're pretty large and looking at other locales I thought it's not necessary.

@Shinigami92
Copy link
Member

I suggest to add the locale to the PR title

@matthewmayer matthewmayer changed the title feat(locale): location feat(locale): add location for uz_UZ_latin Mar 27, 2024
Shinigami92
Shinigami92 previously approved these changes Mar 27, 2024
View reviewed changes
Copy link
Member

@Shinigami92 Shinigami92 left a comment

Choose a reason for hiding this comment

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

Count me already as approved but fix the change request from @ST-DDT pls

@matthewmayer
Copy link
Contributor

Please sort the files alphabetically.

Do you want the street names to be sorted alphabetically? They're pretty large and looking at other locales I thought it's not necessary.

All new locale data should be sorted alphabetically unless there's a good reason (eg months of the year can be kept as Jan-Feb-mar...)

ST-DDT
ST-DDT requested changes Mar 30, 2024
View reviewed 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.

Please make the requested changes.

@ST-DDT ST-DDT added this to the vAnytime milestone Mar 30, 2024
@ST-DDT ST-DDT added p: 1-normal Nothing urgent c: locale Permutes locale definitions m: location Something is referring to the location module labels Mar 30, 2024
@Mirazyzz
PR review fixes
2a25a59 
Fix incorrect country name, use proper prefix for street pattern, add additional secondary pattern
@Mirazyzz
rename street_name to street_name_part
ed4373c
@Mirazyzz
Copy link
Contributor Author

Mirazyzz commented Mar 31, 2024 •
edited

Please make the requested changes.

Ready! sorry for the delay.

ST-DDT
ST-DDT previously approved these changes Mar 31, 2024
View reviewed changes
@ST-DDT ST-DDT requested review from matthewmayer, Shinigami92 and a team March 31, 2024 18:20
Shinigami92
Shinigami92 previously approved these changes Mar 31, 2024
View reviewed changes
@Mirazyzz Mirazyzz dismissed stale reviews from Shinigami92 and ST-DDT via de077ff April 2, 2024 00:51
@ST-DDT ST-DDT merged commit 80730e9 into faker-js:next Apr 2, 2024
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@matthewmayer matthewmayer matthewmayer approved these changes

@ST-DDT ST-DDT ST-DDT approved these changes

@Shinigami92 Shinigami92 Shinigami92 approved these changes

Assignees

@Mirazyzz Mirazyzz

Labels
c: locale Permutes locale definitions m: location Something is referring to the location module p: 1-normal Nothing urgent
Projects
None yet
Milestone
vAnytime
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@Mirazyzz @Shinigami92 @matthewmayer @ST-DDT