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

Add 'Locality' faker #401

Merged
merged 2 commits into from
Sep 27, 2022
Merged

Conversation

panilya
Copy link
Collaborator

@panilya panilya commented Sep 26, 2022

No description provided.

@codecov-commenter
Copy link

codecov-commenter commented Sep 26, 2022

Codecov Report

Merging #401 (326a2b0) into main (eecde32) will increase coverage by 0.05%.
The diff coverage is 100.00%.

@@             Coverage Diff              @@
##               main     #401      +/-   ##
============================================
+ Coverage     93.76%   93.81%   +0.05%     
- Complexity     1978     1980       +2     
============================================
  Files           218      218              
  Lines          3994     3995       +1     
  Branches        386      386              
============================================
+ Hits           3745     3748       +3     
- Misses          149      151       +2     
+ Partials        100       96       -4     
Impacted Files Coverage Δ
...rc/main/java/net/datafaker/base/BaseProviders.java 100.00% <100.00%> (ø)
src/main/java/net/datafaker/base/Locality.java 96.87% <100.00%> (ø)
.../main/java/net/datafaker/service/FakerContext.java 84.61% <0.00%> (-2.57%) ⬇️
...main/java/net/datafaker/service/RandomService.java 94.44% <0.00%> (+2.77%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@panilya
Copy link
Collaborator Author

panilya commented Sep 26, 2022

@snuyanzin Is it okay to do it this way?

README.md Outdated Show resolved Hide resolved
@panilya
Copy link
Collaborator Author

panilya commented Sep 26, 2022

I can't find the cause of this test failure

@panilya panilya changed the title Add 'Locale' faker Add 'Locality' faker Sep 26, 2022
@snuyanzin
Copy link
Collaborator

I created a PR to your branch fixing it panilya#2

@bodiam
Copy link
Contributor

bodiam commented Sep 26, 2022

There might be a bug here, I'm not 100% sure. The test failed because of the repeatability of the locality:

Expected:

 "locality.displayName"="Russian",
 "locality.localeString"="en-ZA",
 "locality.localeStringWithoutReplacement"="fi-FI",

Actual:

 "locality.displayName"="Armenian",
 "locality.localeString"="vi",
 "locality.localeStringWithoutReplacement"="ru",

Or was this the thing you fixed @snuyanzin , and it just hasn't been merged into the PR yet?

@bodiam
Copy link
Contributor

bodiam commented Sep 26, 2022

Ah, it seems all is good now! Thanks both of you!

@panilya
Copy link
Collaborator Author

panilya commented Sep 27, 2022

I think it's ready to merge

@bodiam bodiam merged commit f5ba992 into datafaker-net:main Sep 27, 2022
@bodiam
Copy link
Contributor

bodiam commented Sep 27, 2022

Then let's merge it :-) Nice one, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants