-
-
Notifications
You must be signed in to change notification settings - Fork 973
feat(locale): Add bn_BD locale #3439
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
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 #3439 +/- ##
==========================================
- Coverage 99.97% 99.97% -0.01%
==========================================
Files 2811 2817 +6
Lines 217414 217493 +79
Branches 951 951
==========================================
+ Hits 217361 217438 +77
- Misses 53 55 +2
🚀 New features to boost your workflow:
|
In order to make it easier to review new locales, we generally ask for pull requests to contain a smaller number of files, and focus on a single module at a time. Could I suggest you make a new pull request which just contains
Once that is approved you can follow up with additional PRs to add the remaining data. Also we should decide if we want the locale code to be bn or bn_BD (Bangladesh) - since the phone numbers etc (and the cities?) are country-specific, it should probably be |
@matthewmayer Sure! I’ve updated the PR based on your feedback:
|
…to bn_BD, added 'Beng' to locale-spec
@matthewmayer Hey, made the changes you said. Although after running This PR might've become a little bit too messy for review. If you encounter any issues or suggest any changes, let me know. Also if this PR is becoming troublesome, I can open another PR with the changes you mentioned. |
It's fine. Let's keep it in one PR. We always squash-merge when approving so all the little tweaks don't appear in the main history. I recommend looking at the files view on GitHub to check all changes for your PR. Then it's easier to see if there are any accidental changes that need reverting. Thanks for your contributions! |
@matthewmayer I reverted the changes as you mentioned. Hopefully it should be okay now. |
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.
Thank you for your contribution and the great cooperation with matt ❤️
I'll leave the PR open for at least another 24 hours, so other team members have the chance of reviewing as well.
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.
LGTM (does not include a bitcoin miner 😜)
@xDivisionByZerox Thank you, glad to be of some help<3 |
@Shinigami92 Haha, relieved it passed the crypto-miner test! 😆 |
@AbrarShahriar you can follow up with additional PRs for more modules now For ease of review please make sure
|
@matthewmayer Thanks for the guidance! |
Suggest a new branch for each change based on the current HEAD of next. |
Added Bengali locale.
Modified Modules: