-
Notifications
You must be signed in to change notification settings - Fork 46
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
front: fix rounded hours #10766
front: fix rounded hours #10766
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## dev #10766 +/- ##
==========================================
- Coverage 82.53% 82.53% -0.01%
==========================================
Files 1086 1086
Lines 107928 107931 +3
Branches 740 740
==========================================
- Hits 89080 89077 -3
- Misses 18806 18812 +6
Partials 42 42
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
8e03a12
to
2d9c0c1
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.
Weird, I think dayjs is already expected to handle this case?
https://day.js.org/docs/en/customization/relative-time#relative-time-thresholds-and-rounding
dayjs has built-in rounding behaviors, but by default, it does not round up minutes when seconds are >=30 |
Right, but it can be configured to do so, right? (See above link) |
you mean something like that ? fc1ebcd |
fc1ebcd
to
3e4460a
Compare
Ew, my bad, it seems like the options I've linked to only apply to the relative time plugin, which we don't use. Ideally we'd use a rounding function (just like Temporal has) but it seems like the library is missing that. Oh well, let's just go with your original solution then, sorry for the noise! |
ac7cd0f
to
f0b3131
Compare
@emersion Done ! |
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!
Ideally these commits would be squashed together so that the tests don't regress in-between the commits (useful when bisecting).
f0b3131
to
e0e456d
Compare
Signed-off-by: Theo Macron <[email protected]>
e0e456d
to
4ece033
Compare
closes #9179