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(location): Support ISO 3166-1 numeric country codes #2325

Merged
merged 13 commits into from
Sep 15, 2023
Merged

feat(location): Support ISO 3166-1 numeric country codes #2325

merged 13 commits into from
Sep 15, 2023

Conversation

RobinvanderVliet
Copy link
Contributor

This merge request resolves issue #2302.

@RobinvanderVliet RobinvanderVliet requested a review from a team as a code owner August 18, 2023 16:09
@RobinvanderVliet RobinvanderVliet changed the title Support ISO 3166-1 numeric as variant for the country code API feat(location): Support ISO 3166-1 numeric as variant for the country code API Aug 18, 2023
@codecov
Copy link

codecov bot commented Aug 18, 2023

Codecov Report

Merging #2325 (7953272) into next (cb33664) will increase coverage by 0.00%.
The diff coverage is 99.64%.

Additional details and impacted files
@@           Coverage Diff           @@
##             next    #2325   +/-   ##
=======================================
  Coverage   99.60%   99.60%           
=======================================
  Files        2793     2793           
  Lines      251568   251586   +18     
  Branches     1091     1097    +6     
=======================================
+ Hits       250572   250599   +27     
+ Misses        969      960    -9     
  Partials       27       27           
Files Changed Coverage Δ
src/definitions/location.ts 0.00% <0.00%> (ø)
src/locales/base/location/country_code.ts 100.00% <100.00%> (ø)
src/locales/de_CH/location/country_code.ts 100.00% <100.00%> (ø)
src/locales/fr_CH/location/country_code.ts 100.00% <100.00%> (ø)
src/modules/location/index.ts 99.19% <100.00%> (+0.01%) ⬆️

... and 1 file with indirect coverage changes

Copy link

@suhaibmujahid suhaibmujahid left a comment

Choose a reason for hiding this comment

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

Thank you, @RobinvanderVliet! LGTM.

src/modules/location/index.ts Outdated Show resolved Hide resolved
@ST-DDT ST-DDT linked an issue Aug 19, 2023 that may be closed by this pull request
@ST-DDT ST-DDT added c: feature Request for new feature p: 1-normal Nothing urgent m: location Something is referring to the location module labels Aug 19, 2023
@ST-DDT ST-DDT added the s: waiting for user interest Waiting for more users interested in this feature label Aug 19, 2023
src/modules/location/index.ts Outdated Show resolved Hide resolved
src/modules/location/index.ts Outdated Show resolved Hide resolved
RobinvanderVliet and others added 3 commits September 10, 2023 13:29
Co-authored-by: DivisionByZero <leyla.jaehnig@gmx.de>
Co-authored-by: Shinigami <chrissi92@hotmail.de>
@ST-DDT
Copy link
Member

ST-DDT commented Sep 10, 2023

There seems to be some issues with the new implementation.

@matthewmayer
Copy link
Contributor

I don't think you can do that kind of object mapping in Typescript. Probably use the switch version.

@Shinigami92
Copy link
Member

I don't think you can do that kind of object mapping in Typescript. Probably use the switch version.

No, just add the alpha2 case, and maybe also a as const

@matthewmayer
Copy link
Contributor

i think the switch version is much nicer to read and makes it clear what the default is

const key = (() => {
     switch(variant) {
       case 'numeric': 
         return 'numeric';
       case 'alpha-3': 
         return 'alpha3';
       case 'alpha-2': 
       default: 
         return 'alpha2';
     }
   })();

Co-authored-by: ST-DDT <ST-DDT@gmx.de>
@ST-DDT ST-DDT changed the title feat(location): Support ISO 3166-1 numeric as variant for the country code API feat(location): Support ISO 3166-1 numeric country codes Sep 15, 2023
@ST-DDT ST-DDT enabled auto-merge (squash) September 15, 2023 19:42
@ST-DDT ST-DDT merged commit 82b779d into faker-js:next Sep 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: feature Request for new feature m: location Something is referring to the location module p: 1-normal Nothing urgent s: waiting for user interest Waiting for more users interested in this feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support ISO 3166-1 numeric as variant for the country code API
6 participants