Incorrect handling of durations in SecToTime() #9306

Closed
opened 2025-11-02 08:34:47 -06:00 by GiteaMirror · 3 comments
Owner

Originally created by @edgar-bonet on GitHub (Aug 1, 2022).

Description

Pull request #18863 introduced the following code

years := duration / (3600 * 24 * 7 * 4 * 12)
months := (duration / (3600 * 24 * 30)) % 12
weeks := (duration / (3600 * 24 * 7)) % 4
days := (duration / (3600 * 24)) % 7

This is wrong for a couple of reasons:

  1. the week count reverts to zero (because of the modulus operation) after 4 weeks, which is not quite the same as a month

  2. 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:

years := (duration / 86400) / 365
months := (duration / 86400 - years * 365)  / 30
weeks := (duration / 86400 - years * 365 - month * 30) / 7
days := duration / 86400 - years * 365 - month * 30 - weeks * 7

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

time_tracker
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

Originally created by @edgar-bonet on GitHub (Aug 1, 2022). ### Description Pull request #18863 introduced [the following code][code] ```go years := duration / (3600 * 24 * 7 * 4 * 12) months := (duration / (3600 * 24 * 30)) % 12 weeks := (duration / (3600 * 24 * 7)) % 4 days := (duration / (3600 * 24)) % 7 ``` This is wrong for a couple of reasons: 1. the week count reverts to zero (because of the modulus operation) after 4 weeks, which is not quite the same as a month 2. 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: ```go years := (duration / 86400) / 365 months := (duration / 86400 - years * 365) / 30 weeks := (duration / 86400 - years * 365 - month * 30) / 7 days := duration / 86400 - years * 365 - month * 30 - weeks * 7 ``` I am not submitting a pull request because I have zero experience with the Go programming language. [code]: ../blob/v1.17.0/modules/util/sec_to_time.go#L21-L24 ### Gitea Version 1.18.0+dev-196-ge56005f90 ### Can you reproduce the bug on the Gitea demo site? Yes ### Log Gist _No response_ ### Screenshots ![time_tracker](https://user-images.githubusercontent.com/7680786/182157082-c18100dc-bbda-436f-af3f-f5a0670824c9.png) Screenshot from this [test issue](https://try.gitea.io/edgar/test-repo/issues/2). ### Git Version _No response_ ### Operating System _No response_ ### How are you running Gitea? using https://try.gitea.io/ ### Database _No response_
GiteaMirror added the type/bug label 2025-11-02 08:34:47 -06:00
Author
Owner

@noerw commented on GitHub (Aug 1, 2022):

👀
I think we should just revert https://github.com/go-gitea/gitea/pull/18863 and use https://github.com/hako/durafmt instead, it also supports i18n.

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.

@noerw commented on GitHub (Aug 1, 2022): :eyes: I think we should just revert https://github.com/go-gitea/gitea/pull/18863 and use https://github.com/hako/durafmt instead, it also supports i18n. 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.
Author
Owner

@edgar-bonet commented on GitHub (Aug 1, 2022):

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”.

@edgar-bonet commented on GitHub (Aug 1, 2022): 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][10kh]: no one would call it the “13.7 months rule”. [10kh]: https://en.wikipedia.org/wiki/Outliers_(book)
Author
Owner

@Gusted commented on GitHub (Aug 2, 2022):

@edgar-bonet Thank you for your detailed issue, opened a PR for this at #20610 with the appropriate comments.

@Gusted commented on GitHub (Aug 2, 2022): @edgar-bonet Thank you for your detailed issue, opened a PR for this at #20610 with the appropriate comments.
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: github-starred/gitea#9306