-
Notifications
You must be signed in to change notification settings - Fork 2k
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
[ui] Simplify times in task events #18595
[ui] Simplify times in task events #18595
Conversation
Q for @DingoEatingFuzz : Is the most appropriate place for me to handle this in the TaskEvent model, or in the TaskEvent Serializer? There's already a
aliasing happening in the serializer which I think can probably be removed with this change, but wanted to double check. |
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
Ember Test Audit comparison
|
Personally I see this as presentation logic vs. data logic, so I like it in the model. I could also see it as a helper (though that's also a little odd since it operates on a whole string that happens to have a duration in it, not just a duration). |
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.
Great time to use unit tests, you love to see it!
…age for equality now
192eb9f
to
5daeec8
Compare
Resolves #18507
Task restarts (and possibly other events) use more time specificity than is needed (for example, "task restarting in 10.01934802s").
This simplifies and rounds times to simple second, minute, or hour strings where relevant.