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

Fix hungarian localization #1112

Merged
merged 4 commits into from
Oct 21, 2020
Merged

Fix hungarian localization #1112

merged 4 commits into from
Oct 21, 2020

Conversation

vassbence
Copy link
Contributor

fixes #1111

@codecov
Copy link

codecov bot commented Oct 4, 2020 •
edited
Loading

Codecov Report

Merging #1112 into dev will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##               dev     #1112   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          172       174    +2     
  Lines         1557      1630   +73     
  Branches       330       367   +37     
=========================================
+ Hits          1557      1630   +73
Impacted Files Coverage Δ
src/locale/hu.js 100.00% <100.00%> (ø)
src/locale/br.js 100.00% <0.00%> (ø)
src/locale/ca.js 100.00% <0.00%> (ø)
src/locale/de.js 100.00% <0.00%> (ø)
src/locale/sr.js 100.00% <0.00%> (ø)
src/locale/sr-cyrl.js 100.00% <0.00%> (ø)
src/plugin/objectSupport/index.js 100.00% <0.00%> (ø)
src/plugin/localizedFormat/index.js 100.00% <0.00%> (ø)
src/plugin/localizedFormat/utils.js 100.00% <0.00%> (ø)
src/plugin/arraySupport/index.js 100.00% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7937ccd...d634b10. Read the comment docs.

@tukusejssirs
Copy link
Contributor

tukusejssirs commented Oct 15, 2020 •
edited
Loading

I believe that you need to create some tests for hu.js in test/locale/hu.test.js in order to pass Codecov checks. dayjs strives for 100 % coverage.

Also, the Travis CI builds fail because you does not abide to the eslint config rules set out by the devs; see e.g. this Travis CI build log. Most of them can be fixed from terminal using eslint src/* test/* build/* --fix command, however, it seems like you created this PR using the GitHub website, therefore I am not quite sure you can fix it that way. Anyway, you need to replace double quotes with single quotes (unless you need to escape them, which is not needed in hu.js).

@tukusejssirs
Copy link
Contributor

Great job! As you can see, Travis CI checks passed. Still, you need to create some locale tests in order to increase the code coverage.

@vassbence
Copy link
Contributor Author

Thanks for the help, I added the tests!

tukusejssirs
tukusejssirs approved these changes Oct 15, 2020
View reviewed changes
Copy link
Contributor

@tukusejssirs tukusejssirs left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I reviewed this PR. I have found a few things that might be corrected, however, I don’t this they are merge-blockers.

@iamkun, I can speak Hungarian and I approve the changes this PR.

src/locale/hu.js
dd: (n, s, ___, isFuture) => `${n} ${isFuture || s ? 'nap' : 'napja'}`,
M: (_, s, ___, isFuture) => `egy ${isFuture || s ? 'hónap' : 'hónapja'}`,
MM: (n, s, ___, isFuture) => `${n} ${isFuture || s ? 'hónap' : 'hónapja'}`,
y: (_, s, ___, isFuture) => `egy ${isFuture || s ? 'év' : 'éve'}`,
Copy link
Contributor

Choose a reason for hiding this comment

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

@vassbence, I think that here you could move év right after egy (just like you did with egy perc(e)). That would save you another two chars. 😉

src/locale/hu.js
M: (_, s, ___, isFuture) => `egy ${isFuture || s ? 'hónap' : 'hónapja'}`,
MM: (n, s, ___, isFuture) => `${n} ${isFuture || s ? 'hónap' : 'hónapja'}`,
y: (_, s, ___, isFuture) => `egy ${isFuture || s ? 'év' : 'éve'}`,
yy: (n, s, ___, isFuture) => `${n} ${isFuture || s ? 'év' : 'éve'}`
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here (move év).

test/locale/hu.test.js
const dayjsCompare = dayjs().add(t[0], t[1])
const momentCompare = moment().add(t[0], t[1])

expect(dayjsDay.from(dayjsCompare)).toBe(momentDay.from(momentCompare))
Copy link
Contributor

Choose a reason for hiding this comment

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

I don’t want to nitpick, however, I would remove the double empty lines (keep only one empty line).

@tukusejssirs
Copy link
Contributor

Thanks for the help, I added the tests!

Anytimes! 😃

Not so long ago, I myself was in need of such a help with another PR. 😉

@vassbence vassbence mentioned this pull request Oct 21, 2020
Closed
@iamkun iamkun merged commit ab13754 into iamkun:dev Oct 21, 2020
@vassbence vassbence deleted the patch-1 branch October 21, 2020 09:11
iamkun pushed a commit that referenced this pull request Oct 23, 2020
@semantic-release-bot
chore(release): 1.9.4 [skip ci]
e21aa17 
## [1.9.4](v1.9.3...v1.9.4) (2020-10-23)

### Bug Fixes

* Add descriptions to types ([#1148](#1148)) ([9a407a1](9a407a1))
* add devHelper plugin ([#1163](#1163)) ([de49dc8](de49dc8))
* Fix Hungarian (hu) locale ([#1112](#1112)) ([ab13754](ab13754))
* fix minMax plugin parsing empty array bug ([#1062](#1062)) ([368108b](368108b))
* update adding/subtracting Duration from Dayjs object ([#1156](#1156)) ([f861aca](f861aca))
* update en-NZ locale to use proper ordinal formatting function ([#1143](#1143)) ([fcdbc58](fcdbc58))
* update localeData plugin type ([#1116](#1116)) ([ee5a4ec](ee5a4ec))
* update timezone plugin to support custom parse format ([#1160](#1160)) ([48cbf31](48cbf31)), closes [#1159](#1159)
* update timezone plugin to support keepLocalTime ([#1161](#1161)) ([1d429e5](1d429e5)), closes [#1149](#1149)
@iamkun
Copy link
Owner

iamkun commented Oct 23, 2020

🎉 This PR is included in version 1.9.4 🎉

The release is available on:

Your semantic-release bot 📦🚀

andrewhood125ruhuc added a commit to andrewhood125ruhuc/SidRH2 that referenced this pull request May 10, 2022
@andrewhood125ruhuc
chore(release): 1.9.4 [skip ci]
ba3996b 
## [1.9.4](iamkun/dayjs@v1.9.3...v1.9.4) (2020-10-23)

### Bug Fixes

* Add descriptions to types ([#1148](iamkun/dayjs#1148)) ([9a407a1](iamkun/dayjs@9a407a1))
* add devHelper plugin ([#1163](iamkun/dayjs#1163)) ([de49dc8](iamkun/dayjs@de49dc8))
* Fix Hungarian (hu) locale ([#1112](iamkun/dayjs#1112)) ([ab13754](iamkun/dayjs@ab13754))
* fix minMax plugin parsing empty array bug ([#1062](iamkun/dayjs#1062)) ([368108b](iamkun/dayjs@368108b))
* update adding/subtracting Duration from Dayjs object ([#1156](iamkun/dayjs#1156)) ([f861aca](iamkun/dayjs@f861aca))
* update en-NZ locale to use proper ordinal formatting function ([#1143](iamkun/dayjs#1143)) ([fcdbc58](iamkun/dayjs@fcdbc58))
* update localeData plugin type ([#1116](iamkun/dayjs#1116)) ([ee5a4ec](iamkun/dayjs@ee5a4ec))
* update timezone plugin to support custom parse format ([#1160](iamkun/dayjs#1160)) ([48cbf31](iamkun/dayjs@48cbf31)), closes [#1159](iamkun/dayjs#1159)
* update timezone plugin to support keepLocalTime ([#1161](iamkun/dayjs#1161)) ([1d429e5](iamkun/dayjs@1d429e5)), closes [#1149](iamkun/dayjs#1149)
andrewhood125ruhuc added a commit to andrewhood125ruhuc/SidRH2 that referenced this pull request May 10, 2022
@andrewhood125ruhuc
chore(release): 1.9.4 [skip ci]
b875db0 
## [1.9.4](iamkun/dayjs@v1.9.3...v1.9.4) (2020-10-23)

### Bug Fixes

* Add descriptions to types ([#1148](iamkun/dayjs#1148)) ([9a407a1](iamkun/dayjs@9a407a1))
* add devHelper plugin ([#1163](iamkun/dayjs#1163)) ([de49dc8](iamkun/dayjs@de49dc8))
* Fix Hungarian (hu) locale ([#1112](iamkun/dayjs#1112)) ([ab13754](iamkun/dayjs@ab13754))
* fix minMax plugin parsing empty array bug ([#1062](iamkun/dayjs#1062)) ([368108b](iamkun/dayjs@368108b))
* update adding/subtracting Duration from Dayjs object ([#1156](iamkun/dayjs#1156)) ([f861aca](iamkun/dayjs@f861aca))
* update en-NZ locale to use proper ordinal formatting function ([#1143](iamkun/dayjs#1143)) ([fcdbc58](iamkun/dayjs@fcdbc58))
* update localeData plugin type ([#1116](iamkun/dayjs#1116)) ([ee5a4ec](iamkun/dayjs@ee5a4ec))
* update timezone plugin to support custom parse format ([#1160](iamkun/dayjs#1160)) ([48cbf31](iamkun/dayjs@48cbf31)), closes [#1159](iamkun/dayjs#1159)
* update timezone plugin to support keepLocalTime ([#1161](iamkun/dayjs#1161)) ([1d429e5](iamkun/dayjs@1d429e5)), closes [#1149](iamkun/dayjs#1149)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tukusejssirs tukusejssirs tukusejssirs approved these changes

Assignees
No one assigned
Labels
released
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Improve hungarian localization
3 participants
@vassbence @tukusejssirs @iamkun