-
-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
Incorrect handling of durations in SecToTime() #20589
Comments
👀 edit: The formatted duration is written to the comments table in the DB, so it's not really feasible to translate that in the viewer's locale without reworking the whole thing. So maybe switching to durafmt is not worth it. |
I wonder whether it is really useful to format those times in years, months, weeks and days. In the specific context of recording the time spent working on an issue, I would expect the word “day” to mean a “full work day”. Maybe something around 8 hours or so, but certainly not 24 hours. And a week would likely be 5 days. If someone spent 730 hours working on that issue, “730 hours” would be more suitable than “1 month”. I suggest using hours and minutes as the only time units in this context. Edit: Even when the numbers get large, hours seem to be the preferred unit. See for example the 10,000 hour rule: no one would call it the “13.7 months rule”. |
- Currently there are edge-cases where the code fails to produce the correct results, this is mainly due to the misusage of the modulo operator. Well this could be fixed, it's better to use a more performant way and simply distract the previous number's cost to calculate the variables. - Resolves go-gitea#20589
@edgar-bonet Thank you for your detailed issue, opened a PR for this at #20610 with the appropriate comments. |
- Backport of go-gitea#20610 - Currently there are edge-cases where the code fails to produce the correct results, this is mainly due to the misusage of the modulo operator. Well this could be fixed, it's better to use a more performant way and simply distract the previous number's cost to calculate the variables. - Resolves go-gitea#20589
Description
Pull request #18863 introduced the following code
This is wrong for a couple of reasons:
the week count reverts to zero (because of the modulus operation) after 4 weeks, which is not quite the same as a month
these “years” are 336 days long, which is about 11 months
The consequence is that this function displays:
a duration of 340 days (about 11.2 months) as “1 year 11 months”
a duration of 29 days as “1 day”
a duration of 28 days as the empty string.
See the attached screenshot for the effect of this bug on the display of tracked time.
I suggest to instead use something along these lines:
I am not submitting a pull request because I have zero experience with the Go programming language.
Gitea Version
1.18.0+dev-196-ge56005f90
Can you reproduce the bug on the Gitea demo site?
Yes
Log Gist
No response
Screenshots
Screenshot from this test issue.
Git Version
No response
Operating System
No response
How are you running Gitea?
using https://try.gitea.io/
Database
No response
The text was updated successfully, but these errors were encountered: