-
Notifications
You must be signed in to change notification settings - Fork 45
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
momentjs/date-fns to dayjs #2693
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #2693 +/- ##
==========================================
- Coverage 72.85% 72.73% -0.13%
==========================================
Files 249 250 +1
Lines 8520 8592 +72
Branches 1808 1819 +11
==========================================
+ Hits 6207 6249 +42
- Misses 1705 1727 +22
- Partials 608 616 +8 ☔ View full report in Codecov by Sentry. |
3 failed and 3 flaky tests on run #3883 ↗︎Details:
|
Test | Artifacts | |
---|---|---|
Searches > should edit search |
Screenshots
|
setting/FlowSetting.spec.ts • 1 failed test
Test | Artifacts | |
---|---|---|
Organization Settings > should add default flow |
Screenshots
|
trigger/Trigger.spec.ts • 1 failed test
Test | Artifacts | |
---|---|---|
Triggers (daily) > should create new trigger |
Screenshots
|
flow/Flow.spec.ts • 1 flaky test
Test | Artifacts | |
---|---|---|
Flow > should create duplicate Flow |
Screenshots
|
roles/staff/chat/Chat.spec.ts • 1 flaky test
Test | Artifacts | |
---|---|---|
Role - Staff - Chats > should go to top |
Screenshots
|
staffmanagement/StaffManagement.spec.ts • 1 flaky test
Test | Artifacts | |
---|---|---|
Staff Management > should delete collection |
Screenshots
|
Review all test suite changes for PR #2693 ↗︎
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.
Overall looks good.
Two things:
- We should remove date-fns also with dayjs as mentioned in the issue. There is still a date-fns adapter that is being used
- Remove moment and date-fns packages from package.json
src/common/constants.ts
Outdated
@@ -112,8 +112,8 @@ export const setVariables = ( | |||
}, | |||
}); | |||
|
|||
export const is24HourWindowOver = (time: any) => | |||
moment.duration(moment(new Date()).diff(moment(time))).asHours() > 24; | |||
export const is24HourWindowOver = (time: any) => |
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.
extra space here too.
src/containers/SettingList/OrganizationFlows/OrganisationFlows.tsx
Outdated
Show resolved
Hide resolved
Co-authored-by: Mohd Shamoon <32592458+mdshamoon@users.noreply.github.com>
Co-authored-by: Mohd Shamoon <32592458+mdshamoon@users.noreply.github.com>
Summary
Migrated to dayjs from date-fns / momentjs as now it's a recommended option by MUI.
Check https://mui.com/x/react-date-pickers/#date-library
Datejs: https://day.js.org/
Test Plan