Show expected time left for pending PRs - #788
Conversation
fedb371 to
cfef57d
Compare
| display.textContent = formattedTime; | ||
| const formattedElapsed = formatDuration(elapsedMs); | ||
| const expectedMs = AVERAGE_BUILD_DURATION_MS - elapsedMs; | ||
| const formattedExpected = expectedMs < 0 ? `~${formatDuration(-expectedMs)} overdue` : `~${formatDuration(expectedMs)} left`; |
There was a problem hiding this comment.
Not sure if "overdue" is the best wording. Open to suggestions. Also, is there some way to test the display of the queue locally without setting up a whole fake database etc.?
There was a problem hiding this comment.
Hi, thanks for the PR! I think that showing the remaining time is a good idea, but I'm not sure if we should format it like this, because it can become quite wide:
What do you think about:
- Removing
pending(since that is kinda implied) - Keeping only the expected part, so that it will always show only "1h 3m left" or "5m overdue"
?
|
I also don't love this format (thanks for sending the preview btw -- I don't have a local setup for bors). However, I feel like we should still say pending or something similar. It's obvious to us but might not be to newbies. We could use icons instead of "pending" and "approved". Could be cool but I kinda like the simplicity of using words. Another option is keeping expected if it's "1h 3m left", otherwise say how much is elapsed (and sidestep the whole overdue wording issue as well). E.g. it'd be either "pending (1h 3m left)" or "pending (5h so far)" if it's slow. Though then it's not obvious how much overdue it is. It'd be nice to keep the absolute elapsed time rather than just the relative-to-average delta, though you might be right that just showing the delta makes more sense. |
|
By the way, is the progress bar not enough for showing the expected time left? 🤔 |
|
Not really because then you need to do mental arithmetic. Like if it's 2 hours done and the bar looks around 70% full, I pretty much have to do 2/0.7 - 2, then multiply by 60, if I want to know how many minutes are left. This is why progress bars in terminals and other apps usually show visually how much is done, and the percentage done, and the time spent so far, and the expected time left. Better to make the information easily available. |
|
Showing everything (but still without %) doesn't look great: I think that a compromise would be to show the expected remaining time, that might be a bit more useful than the elapsed time. |
Or show how long since the expected completion time for slow PRs.
|
Thanks, this should be fine visually (I'd add some more CSS to limit the width of the column, but that's a detail). I still wonder about the shown message. We say "N minutes left", but the variability of our CI is 20-30 minutes, so it can be a bit misleading to say "left" and "overdue". But anyway, I think we can try it and see what others think. |
|
To me this seems more confusing and less reliable than just showing the elapsed time. Now there's no way to see how long the current job has been running, which is important information. And the estimated time remaining is simply not reliable, so I don't see it as useful. |
Note that you can still see this information by hovering over the "pending" entry (showing the HTML
It is roughly accurate; there is just some error regarding the exact time remaining. I just don't think that showing only elapsed time is super useful since there isn't a reference point for how long a normal build is. The advantage of this PR too is now it's easier to identify builds that are slower than usual since they are marked as overdue. Open to any suggestions of better UIs for this. |
No, you can't. Try it. There is no way to see the elapsed time. In addition, hovering is a huge pain compared to being able to look at the page, and doesn't work at all on mobile.
It's not accurate at all. The time estimator has no idea when the current job will stop, which could be anywhere between 0 hours and 6 hours from when it started. It can't possibly know.
This change makes it way harder to identify slow builds, because it won't tell me how long the current build has been running! We already had a good UI, and it was the previous one. I'm pretty upset that a successful design was made massively less useful for reasons that make no sense to me. |
|
Zalathar is doing a lot of rollup work and PR scheduling, so I will fully defer to their (and also others') preference on this, and revert this PR (sorry, Noah!). That being said, I also find the expected remaining time to be kinda useful. And this conversation just gave me an idea on how to:
With a single solution. I'll try to work on it today. |
|
Implemented this in #795. It can be later extended to also show pending try builds. |
|
No worries, I like your new approach better anyway. I agree that elapsed time is useful, but like you I'd also like to see estimated time left. |
3 participants
Or show how long since the expected completion time for slow PRs.