-
-
Notifications
You must be signed in to change notification settings - Fork 318
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
fix(locale): dateFns generator for korean #603
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
@zombieJ can you review about this PR? |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #603 +/- ##
=======================================
Coverage 98.57% 98.57%
=======================================
Files 63 63
Lines 2530 2531 +1
Branches 692 667 -25
=======================================
+ Hits 2494 2495 +1
Misses 33 33
Partials 3 3 ☔ View full report in Codecov by Sentry. |
Could you talk about the problems and background of this PR fix? |
@yoyo837 date-fns package do not have |
Could you add some test case for it? |
@nnnnoel is attempting to deploy a commit to the React Component Team on Vercel. A member of the Team first needs to authorize it. |
@yoyo837 Test Added |
test failed. |
@yoyo837 fixed. |
@@ -6,6 +6,7 @@ import luxonGenerateConfig from '../src/generate/luxon'; | |||
import { getMoment } from './util/commonUtil'; | |||
|
|||
import 'dayjs/locale/zh-cn'; | |||
import 'dayjs/locale/ko'; |
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.
Seems change in generate/dateFns.ts
should not affect dayjs, is this import necessary?
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.
dayjs locale js file should be imported before use.
cause of dynamic register locale config
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.
The problem is that the fix for dateFNS should not be covered using dayjs test cases.
No description provided.