-
Notifications
You must be signed in to change notification settings - Fork 901
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
Fix displaying ETA output for download tracker, make it consistent & treat output as integers. #1832
Conversation
I thought we'd already dealt with this, sigh. What types are involved here because really all of the days/hours/minutes/seconds should be integers. |
@rbtcollins Could you take a look and ensure that your PRs would fix this too? Assuming so, let me know and we'll close this in favour of the more deep correction. If not, then if I merge this, could you update your PRs to match? |
☔ The latest upstream changes (presumably #1836) made this pull request unmergeable. Please resolve the merge conflicts. |
My PR didn't fix this sorry, I was fixing an orthogonal problem - this change is still needed; @konstankinollc could you please rebase your patch then @kinnison can merge it? Thanks! |
Absolutely, will do in a bit. |
@kinnison ready for your review |
It also addresses a bug in computing hours to download. |
Can anyone please explain why the build is failing on CI? |
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.
As noted, please don't update the package version in Cargo.toml
and I've explained the CI failure for you
9c45e86
to
58408ea
Compare
Make download duration output consistent & treat as integers.
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 now, if it passes CI then it can merge IMO
Addresses #1831