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): and location to fr_SN #2533

Merged
merged 10 commits into from Nov 11, 2023

Conversation

makhtar-sarr
Copy link
Contributor

In this PR I add the most popular cities in Senegal as well as the zip code format.

https://en.wikipedia.org/wiki/List_of_cities_in_Senegal

@makhtar-sarr makhtar-sarr requested a review from a team as a code owner November 8, 2023 13:25
Copy link

codecov bot commented Nov 8, 2023

Codecov Report

Merging #2533 (f3161cd) into next (fa26a44) will decrease coverage by 0.01%.
The diff coverage is 100.00%.

Additional details and impacted files
@@            Coverage Diff             @@
##             next    #2533      +/-   ##
==========================================
- Coverage   99.58%   99.57%   -0.01%     
==========================================
  Files        2787     2794       +7     
  Lines      249376   249473      +97     
  Branches     1084     1083       -1     
==========================================
+ Hits       248333   248425      +92     
- Misses       1015     1020       +5     
  Partials       28       28              
Files Coverage Δ
src/locales/fr_SN/index.ts 100.00% <100.00%> (ø)
src/locales/fr_SN/location/building_number.ts 100.00% <100.00%> (ø)
src/locales/fr_SN/location/city_name.ts 100.00% <100.00%> (ø)
src/locales/fr_SN/location/city_pattern.ts 100.00% <100.00%> (ø)
src/locales/fr_SN/location/default_country.ts 100.00% <100.00%> (ø)
src/locales/fr_SN/location/index.ts 100.00% <100.00%> (ø)
src/locales/fr_SN/location/postcode.ts 100.00% <100.00%> (ø)
src/locales/fr_SN/location/state.ts 100.00% <100.00%> (ø)

... and 2 files with indirect coverage changes

ST-DDT
ST-DDT previously approved these changes Nov 8, 2023
@ST-DDT ST-DDT requested review from a team November 8, 2023 14:04
@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 Nov 8, 2023
@ST-DDT ST-DDT added this to the vAnytime milestone Nov 8, 2023
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.

How about location.state() does it make sense to use the regions for that?

https://en.wikipedia.org/wiki/Subdivisions_of_Senegal?wprov=sfti1#

@makhtar-sarr
Copy link
Contributor Author

Yes, that's a good idea, but should I remove them from the list of cities or not?

@matthewmayer
Copy link
Contributor

In other locales where state names happen to duplicate city names, we normally leave them in.

@ST-DDT ST-DDT requested review from a team November 9, 2023 19:51
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.

Looks good to me. Location module now seems well covered, could consider adding another PR with improved street names (otherwise will inherit from fr which might not be ideal).

@ST-DDT ST-DDT merged commit f730125 into faker-js:next Nov 11, 2023
20 checks passed
@ST-DDT
Copy link
Member

ST-DDT commented Nov 11, 2023

Thanks for your contribution ❤️ .

@makhtar-sarr makhtar-sarr deleted the add-locale-senegal branch November 11, 2023 09:54
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.

None yet

3 participants