-
-
Notifications
You must be signed in to change notification settings - Fork 942
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): add list of spoken languages #3333
Conversation
✅ Deploy Preview for fakerjs ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify site configuration. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## next #3333 +/- ##
==========================================
- Coverage 99.97% 99.96% -0.01%
==========================================
Files 2807 2809 +2
Lines 217087 217202 +115
Branches 969 967 -2
==========================================
+ Hits 217029 217134 +105
- Misses 58 68 +10
|
I think based on discussion at #2622 we wanted this to return an object with not just the language name but also the iso 2 and 3 letter codes where available. |
Thanks @matthewmayer I didn't read that discussion, I'll update it later tonight |
slim them down to a few languages as the long list was not easy to build up
It's ready for review |
This reverts commit 72a18e9.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your quick responses.
I have found a few more things to potentially improve this.
My biggest question mark regarding this feature is which values the method is supposed to return.
- The languages commonly known/spoken by/relevant for the populance of a locale
- "All" languages regardless of applicability for the locale (current)
(This is relevant for the countryCode method as well #2305)
I'd say all languages. |
I think we should include at least all languages faker is localised into. I think the following are missing.
|
Co-authored-by: ST-DDT <ST-DDT@gmx.de>
Co-authored-by: ST-DDT <ST-DDT@gmx.de>
Co-authored-by: ST-DDT <ST-DDT@gmx.de>
Co-authored-by: ST-DDT <ST-DDT@gmx.de>
I have pushed the latest changes. @ST-DDT @matthewmayer thanks a lot guys for going through the changes in smaller iterations! for #2305 I'm not sure how to approach it, do I hard coded which |
yeah dont worry about #2305 for now that would certainly be a different PR if we decide to do it |
Co-authored-by: ST-DDT <ST-DDT@gmx.de>
closes #1548
closes #2622
For Ref:
Languages added for