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 minimal da locale, start with location #2324

Merged
merged 2 commits into from
Aug 19, 2023

Conversation

stonor
Copy link
Contributor

@stonor stonor commented Aug 18, 2023

As suggested in #2321, add new language in smaller chunks.

@stonor stonor requested a review from a team August 18, 2023 10:47
@stonor stonor requested a review from a team as a code owner August 18, 2023 10:47
@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 Aug 18, 2023
@codecov
Copy link

codecov bot commented Aug 18, 2023

Codecov Report

Merging #2324 (b02b490) into next (aa1bb13) will decrease coverage by 0.01%.
The diff coverage is 100.00%.

Additional details and impacted files
@@            Coverage Diff             @@
##             next    #2324      +/-   ##
==========================================
- Coverage   99.60%   99.59%   -0.01%     
==========================================
  Files        2644     2660      +16     
  Lines      245613   246345     +732     
  Branches     1084     1082       -2     
==========================================
+ Hits       244640   245357     +717     
- Misses        946      961      +15     
  Partials       27       27              
Files Changed Coverage Δ
src/locale/da.ts 100.00% <100.00%> (ø)
src/locale/index.ts 100.00% <100.00%> (ø)
src/locales/da/index.ts 100.00% <100.00%> (ø)
src/locales/da/location/building_number.ts 100.00% <100.00%> (ø)
src/locales/da/location/city_name.ts 100.00% <100.00%> (ø)
src/locales/da/location/city_pattern.ts 100.00% <100.00%> (ø)
src/locales/da/location/country.ts 100.00% <100.00%> (ø)
src/locales/da/location/default_country.ts 100.00% <100.00%> (ø)
src/locales/da/location/direction.ts 100.00% <100.00%> (ø)
src/locales/da/location/direction_abbr.ts 100.00% <100.00%> (ø)
... and 8 more

... and 1 file with indirect coverage changes

Copy link
Contributor

@matthewmayer matthewmayer left a comment

Choose a reason for hiding this comment

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

Should we consider making this locale da_DK as Danish is spoken in some other territories with their own ISO codes like Greenland or Faroe Islands?

@ST-DDT
Copy link
Member

ST-DDT commented Aug 18, 2023

Should we consider making this locale da_DK as Danish is spoken in some other territories with their own ISO codes like Greenland or Faroe Islands?

No, because these other territories probably share a lot of the names/values with Denmark and we shouldn't duplicate them.
If there are da_DK specific things then we can still split or re-arrange them then.

@ST-DDT ST-DDT requested review from a team August 18, 2023 11:57
@matthewmayer
Copy link
Contributor

I'm not sure when we do lang vs lang_COUNTRY though. Eg ka_GE and sr_RS were added recently

@ST-DDT ST-DDT merged commit c158b38 into faker-js:next Aug 19, 2023
19 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: location Something is referring to the location module p: 1-normal Nothing urgent
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants