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.

Sign up for GitHub

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

Jump to bottom

feat: add Hungarian locale #53

Merged
merged 2 commits into from
Apr 22, 2022
Merged

feat: add Hungarian locale #53

merged 2 commits into from
Apr 22, 2022

Conversation

szabolcstarnai
Copy link
Contributor

Added Hungarian locale for (some) animals, commerce, date, finance, internet, name, phone_number and word

damienwebdev
damienwebdev previously requested changes Jan 10, 2022
View reviewed changes
Copy link
Member

@damienwebdev damienwebdev left a comment

Choose a reason for hiding this comment

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

We will need someone who speaks Hungarian (other than the author) to review this prior to merging.

.gitignore Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
lib/locales/hu/animal/index.js Outdated Show resolved Hide resolved
@Shinigami92 Shinigami92 added the p: 1-normal Nothing urgent label Jan 10, 2022
@Shinigami92 Shinigami92 self-requested a review January 10, 2022 16:38
Shinigami92
Shinigami92 requested changes Jan 12, 2022
View reviewed changes
Copy link
Member

@Shinigami92 Shinigami92 left a comment

Choose a reason for hiding this comment

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

Just in case, please revert examples/browser/js/faker.js and examples/browser/js/faker.min.js
We will keep track of that on our side

@netlify
Copy link

netlify bot commented Jan 14, 2022 •
edited
Loading

✔️ Deploy Preview for vigilant-wescoff-04e480 ready!

🔨 Explore the source changes: f03239b

🔍 Inspect the deploy log: https://app.netlify.com/sites/vigilant-wescoff-04e480/deploys/61eb3d5e356bc90007714a62

😎 Browse the preview: https://deploy-preview-53--vigilant-wescoff-04e480.netlify.app

@szabolcstarnai szabolcstarnai changed the title Adding Hungarian locale feat: Adding Hungarian locale Jan 14, 2022
@prisis prisis added this to the v6.1.0 milestone Jan 16, 2022
@szabolcstarnai szabolcstarnai requested a review from a team as a code owner January 19, 2022 20:50
Shinigami92
Shinigami92 requested changes Jan 21, 2022
View reviewed changes
Copy link
Member

@Shinigami92 Shinigami92 left a comment

Choose a reason for hiding this comment

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

We now have all locales written in TypeScript.
Please have a look and rewrite your files like we have done with all other files.

@Shinigami92
Copy link
Member

@patak-dev I hope I didn't miss-assume your language 😅
But could you have a quick look about this?

That would be really nice

@patak-dev
Copy link

I'm Argentinian @Shinigami92, not the best for the task 😅

@Shinigami92
Copy link
Member

I'm Argentinian @Shinigami92, not the best for the task sweat_smile

Damnd, okay, sorry about that. Hope it's not to cold in the snow ❄️ 🏂
Take care 👋

szabolcstarnai
szabolcstarnai commented Jan 24, 2022
View reviewed changes
Copy link
Contributor Author

@szabolcstarnai szabolcstarnai left a comment

Choose a reason for hiding this comment

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

In Hungarian 'prepositions' don't really exist, practically we only use postpositions.

@import-brain import-brain added the c: feature Request for new feature label Feb 24, 2022
@Shinigami92 Shinigami92 added c: locale Permutes locale definitions and removed c: feature Request for new feature labels Mar 18, 2022
@codecov
Copy link

codecov bot commented Apr 5, 2022 •
edited
Loading

Codecov Report

Merging #53 (715b26f) into main (cbc99d0) will increase coverage by 0.07%.
The diff coverage is 99.36%.

@@            Coverage Diff             @@
##             main      #53      +/-   ##
==========================================
+ Coverage   99.34%   99.42%   +0.07%     
==========================================
  Files        1922     1958      +36     
  Lines      183057   210773   +27716     
  Branches      903      904       +1     
==========================================
+ Hits       181860   209557   +27697     
- Misses       1141     1159      +18     
- Partials       56       57       +1
Impacted Files Coverage Δ
src/locale/hu.ts 0.00% <0.00%> (ø)
src/locales/hu/word/adjective.ts 100.00% <ø> (ø)
src/locales/hu/word/noun.ts 100.00% <ø> (ø)
src/locales/hu/word/verb.ts 100.00% <ø> (ø)
src/locales/hu/animal/cat.ts 100.00% <100.00%> (ø)
src/locales/hu/animal/dog.ts 100.00% <100.00%> (ø)
src/locales/hu/animal/horse.ts 100.00% <100.00%> (ø)
src/locales/hu/animal/index.ts 100.00% <100.00%> (ø)
src/locales/hu/commerce/color.ts 100.00% <100.00%> (ø)
src/locales/hu/commerce/department.ts 100.00% <100.00%> (ø)
... and 27 more

@szabolcstarnai szabolcstarnai requested a review from a team April 5, 2022 16:07
Shinigami92
Shinigami92 previously approved these changes Apr 5, 2022
View reviewed changes
Copy link
Member

@Shinigami92 Shinigami92 left a comment

Choose a reason for hiding this comment

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

We need someone that can approve this PR in terms of Hungarian

@import-brain import-brain added the s: accepted Accepted feature / Confirmed bug label Apr 11, 2022
@Shinigami92 Shinigami92 changed the title feat: Adding Hungarian locale feat: add Hungarian locale Apr 22, 2022
@Shinigami92 Shinigami92 requested review from a team April 22, 2022 10:32
ST-DDT
ST-DDT approved these changes Apr 22, 2022
View reviewed changes
Copy link
Member

@ST-DDT ST-DDT 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 format wise.

@Shinigami92 Shinigami92 enabled auto-merge (squash) April 22, 2022 10:32
@Shinigami92
Copy link
Member

If someone else uses these, the new locales and detect that something is wrong, they can raise a new issue / open new PR

@Shinigami92 Shinigami92 merged commit 9b0d100 into faker-js:main Apr 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Shinigami92 Shinigami92 Shinigami92 approved these changes

@ST-DDT ST-DDT ST-DDT approved these changes

@damienwebdev damienwebdev Awaiting requested review from damienwebdev

Assignees

@szabolcstarnai szabolcstarnai

Labels
c: locale Permutes locale definitions p: 1-normal Nothing urgent s: accepted Accepted feature / Confirmed bug
Projects
None yet
Milestone
v6.3 - Next Minor
Development

Successfully merging this pull request may close these issues.
7 participants
@szabolcstarnai @Shinigami92 @patak-dev @ST-DDT @damienwebdev @prisis @import-brain