Bug 1834262: show relative time for pipeline run duration #5378
Conversation
|
/retitle Bug 1834262: show relative time for pipeline run duration |
openshift-ci-robot
changed the title
openshift-ci-robot
added
bugzilla/severity-low
|
@vikram-raj: This pull request references Bugzilla bug 1834262, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker. 3 validation(s) were run on this bug
In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
an existing testcase needs to be fixed. |
vikram-raj
force-pushed
the
odc-2239
branch
3 times, most recently
from
507d199
to
aae60a3
Compare
| @@ -314,6 +314,30 @@ export const getPipelineRunWorkspaces = ( | |||
| ); | |||
| }; | |||
|
|
|||
| export const calculateRelativeTime = (startTime: string, completionTime?: string) => { | |||
There was a problem hiding this comment.
Please write stand alone tests for this @vikram-raj Not sure why you updated the other test and didn't write more for just this method :)
Not sure if this would have caught your logic gap but it might have.
| if (minutesAgo > 90) { | ||
| const count = Math.round(hoursAgo); | ||
| return `about ${count} hours`; | ||
| } | ||
| if (minutesAgo > 45) { |
There was a problem hiding this comment.
What about the 'about an hour'?
More than 45m to less than 90m can have 1h 29m 59s be 'less than a hour'?
| return `about ${count} hours`; | ||
| } | ||
| if (minutesAgo > 45) { | ||
| return 'less than a hour'; |
There was a problem hiding this comment.
English is stupid lol. I read this and felt it was wrong, but I didn't want to say that without finding a reason for it. I think this qualities:
First thing first, to decide whether it’s a/an to a word, it all depends on how we pronounce the word itself. If we pronounce the word, and the first sound starts with vowels (a,i,u,o,e) you give ‘an’. [...]
For the word ‘Hour’, the correct pronunciation is /auer/ more or less. The first letter we hear is ‘a’ which is a vowel, so we put ‘an hour’ not ‘a hour’. I am giving you more example. Let’s take a look at the word ‘University’, to pronounce it, you say /yuniversiti/. Having said that, the very first letter you her is /y/ so you do not put ‘an university’, but ‘a university’.
[source]
| return 'less than a hour'; | |
| return 'less than an hour'; |
| const count = Math.round(minutesAgo); | ||
| return `about ${count} minutes`; | ||
| } | ||
| if (secondsAgo > 45) { |
There was a problem hiding this comment.
Same issue here... this will be 'less than a minute' for up to 1m 29s.
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: andrewballantyne, vikram-raj The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci-robot
added
the
approved
|
@vikram-raj: All pull requests linked via external trackers have merged: openshift/console#5378. Bugzilla bug 1834262 has been moved to the MODIFIED state. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
Fixes:
https://issues.redhat.com/browse/ODC-2239
Analysis / Root cause:
When we are showing live duration progress, we are at the whims of the firehose kicks. This can cause our data to not live update, but jump several seconds forward.
Solution Description:
Instead of showing the exact time. Now, we show the pipeline run duration in relative time formate.
Screen shots / Gifs for design review:
cc: @openshift/team-devconsole-ux
Browser conformance: