-
Notifications
You must be signed in to change notification settings - Fork 521
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
Moment.js replaced with Dayjs #1738
Moment.js replaced with Dayjs #1738
Conversation
Codecov ReportPatch coverage is 📢 Thoughts on this report? Let us know!. |
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.
can you add more tests to raise util/date coverage to 100%?
packages/jaeger-ui/src/components/SearchTracePage/SearchForm.test.js
Outdated
Show resolved
Hide resolved
packages/jaeger-ui/src/components/SearchTracePage/SearchResults/ResultItem.tsx
Outdated
Show resolved
Hide resolved
I was on antd migration guide from v3 to v4 and I noticed, they provided a guide to migrate from moment to dayjs. Have you used it to achieve this ? |
Hi @mmorel-35, the guide you mentioned is specifically for Ant Design component, this is not specific to moment to dayjs guide and thus not relevant to our scenario |
I forgot to add more unit tests, let me do it real quick |
@prathamesh-mutkure please add missing tests to have patch coverage 100% |
c1af184
to
2a71e75
Compare
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.
LGTM!
Can you rebase your changes with the latest main branch? |
7cbdced
to
5f76300
Compare
1b75383
to
1095a96
Compare
Signed-off-by: Prathamesh Mutkure <pmutkure009@gmail.com>
Signed-off-by: Prathamesh Mutkure <pmutkure009@gmail.com>
Signed-off-by: Prathamesh Mutkure <pmutkure009@gmail.com>
Signed-off-by: Prathamesh Mutkure <pmutkure009@gmail.com>
Signed-off-by: Prathamesh Mutkure <pmutkure009@gmail.com>
Signed-off-by: Prathamesh Mutkure <pmutkure009@gmail.com>
Signed-off-by: Prathamesh Mutkure <pmutkure009@gmail.com>
Signed-off-by: Prathamesh Mutkure <pmutkure009@gmail.com>
Signed-off-by: Prathamesh Mutkure <pmutkure009@gmail.com>
Signed-off-by: Prathamesh Mutkure <pmutkure009@gmail.com>
Signed-off-by: Prathamesh Mutkure <pmutkure009@gmail.com>
9c33052
to
492e9aa
Compare
## Which problem is this PR solving? - Bunch of new constants are added as exported in #1738, even though they are not used anywhere in the code ## Description of the changes - Remove `export` ## How was this change tested? - CI --------- Signed-off-by: Yuri Shkuro <github@ysh.us>
Which problem is this PR solving?
Description of the changes
moment.js
and replaced it withdayjs
packagesHow was this change tested?
Monitor
tab,Span
andTrace
view,Search
page, all the charts, etc.The UI was checked between
jaeger-all-in-one
and changes of this commitChecklist
jaeger
:make lint test
jaeger-ui
:yarn lint
andyarn test