Skip to content
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 SecToTime edge-cases #20610

Merged
merged 12 commits into from
Aug 8, 2022
Merged

Fix SecToTime edge-cases #20610

merged 12 commits into from
Aug 8, 2022

Conversation

Gusted
Copy link
Contributor

@Gusted Gusted commented Aug 2, 2022

  • 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 Incorrect handling of durations in SecToTime() #20589

- 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
@Gusted Gusted added this to the 1.18.0 milestone Aug 2, 2022
@Gusted Gusted added backport/v1.17 issue/regression Issue needs no code to be fixed, only a description on how to fix it yourself labels Aug 2, 2022
modules/util/sec_to_time_test.go Outdated Show resolved Hide resolved
modules/util/sec_to_time.go Show resolved Hide resolved
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Aug 2, 2022
Gusted added a commit to Gusted/gitea that referenced this pull request Aug 2, 2022
- 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
@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Aug 2, 2022
@Gusted Gusted added the backport/done All backports for this PR have been created label Aug 2, 2022
modules/util/sec_to_time_test.go Outdated Show resolved Hide resolved
modules/util/sec_to_time_test.go Outdated Show resolved Hide resolved
modules/util/sec_to_time.go Outdated Show resolved Hide resolved
modules/util/sec_to_time.go Outdated Show resolved Hide resolved
modules/util/sec_to_time.go Outdated Show resolved Hide resolved
@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Aug 2, 2022
@edgar-bonet
Copy link

It appears that the following test failed in modules/util/sec_to_time_test.go:

assert.Equal(t, "11 months 1 week", SecToTime(year-25*day))

This is because:

  • year-25*day is 340 days
  • 11 months is 11*365/12 = 334 days

thus, year-25*day is 11 months and 6 days, not quite 11 months and one week.

@delvh
Copy link
Member

delvh commented Aug 7, 2022

I mean, now it's at least easy to adapt the tests.
What would be preferred? Changing the expectation to 11 months, or subtracting 24*day instead?

@Gusted
Copy link
Contributor Author

Gusted commented Aug 7, 2022

I mean, now it's at least easy to adapt the tests.
What would be preferred? Changing the expectation to 11 months, or subtracting 24*day instead?

Given the previous refactor new behavior, it should only display months + weeks in this case. Which means the 11 months is technically the correct one in this case. I'll adjust the code

@wxiaoguang wxiaoguang merged commit 498352c into go-gitea:main Aug 8, 2022
wxiaoguang pushed a commit that referenced this pull request Aug 8, 2022
zjjhot added a commit to zjjhot/gitea that referenced this pull request Aug 9, 2022
* giteaofficial/main:
  Add support for `npm unpublish` (go-gitea#20688)
  Allow multiple files in generic packages (go-gitea#20661)
  Refactor legacy git init (go-gitea#20376)
  Fix typo in source (go-gitea#20723)
  [skip ci] Updated translations via Crowdin
  Add issue filter for Author (go-gitea#20578)
  Fix init mail render logic (go-gitea#20704)
  Frontport changelog v1.17.0 (go-gitea#20712)
  Fix disable download button (go-gitea#20701)
  docs: move search input to navbar (go-gitea#20551)
  Fix SecToTime edge-cases (go-gitea#20610)
vsysoev pushed a commit to IntegraSDL/gitea that referenced this pull request Aug 10, 2022
zeripath added a commit to zeripath/gitea that referenced this pull request Aug 17, 2022
 ## [1.17.1](https://github.com/go-gitea/gitea/releases/tag/1.17.1) - 2022-08-17

* SECURITY
  * Correctly escape within tribute.js (go-gitea#20831) (go-gitea#20832)
* FEATURES
  * Add support for NuGet API keys (go-gitea#20721) (go-gitea#20734)
* ENHANCEMENTS
  * Display project in issue list (go-gitea#20583)
  * Add disable download source configuration (go-gitea#20548) (go-gitea#20579)
* BUGFIXES
  * Use the total issue count for UI (go-gitea#20785) (go-gitea#20827)
  * Add proxy host into allow list (go-gitea#20798) (go-gitea#20819)
  * Add missing translation for queue flush workers (go-gitea#20791) (go-gitea#20792)
  * Improve comment header for mobile (go-gitea#20781) (go-gitea#20789)
  * Fix git.Init for doctor sub-command (go-gitea#20782) (go-gitea#20783)
  * Check webhooks slice length before calling xorm (go-gitea#20642) (go-gitea#20768)
  * Remove manual rollback for failed generated repositories (go-gitea#20639) (go-gitea#20762)
  * Use correct field name in npm template (go-gitea#20675) (go-gitea#20760)
  * Keep download count on Container tag overwrite (go-gitea#20728) (go-gitea#20735)
  * Fix v220 migration to be compatible for MSSQL 2008 r2 (go-gitea#20702) (go-gitea#20707)
  * Use request timeout for git service rpc (go-gitea#20689) (go-gitea#20693)
  * Send correct NuGet status codes (go-gitea#20647) (go-gitea#20677)
  * Use correct context to get package content (go-gitea#20673) (go-gitea#20676)
  * Fix the JS error "EventSource is not defined" caused by some non-standard browsers (go-gitea#20584) (go-gitea#20663)
  * Add default commit messages to PR for squash merge (go-gitea#20618) (go-gitea#20645)
  * Fix package upload for files >32mb (go-gitea#20622) (go-gitea#20635)
  * Fix the new-line copy-paste for rendered code (go-gitea#20612)
  * Clean up and fix clone button script (go-gitea#20415 & go-gitea#20600) (go-gitea#20599)
  *  Fix default merge style (go-gitea#20564) (go-gitea#20565)
  * Add repository condition for issue count (go-gitea#20454) (go-gitea#20496)
* MISC
  * Make branch icon stand out more (go-gitea#20726) (go-gitea#20774)
  * Fix loading button with invalid form (go-gitea#20754) (go-gitea#20759)
  * Add username check to doctor (go-gitea#20140) (go-gitea#20671)
  * Enable Wire 2 for Internal SSH Server (go-gitea#20616) (go-gitea#20617)
  *  Fix SecToTime edge-cases (go-gitea#20610) (go-gitea#20611)

Signed-off-by: Andrew Thornton <art27@cantab.net>
@zeripath zeripath mentioned this pull request Aug 17, 2022
techknowlogick added a commit that referenced this pull request Aug 18, 2022
* Changelog 1.17.1

 ## [1.17.1](https://github.com/go-gitea/gitea/releases/tag/1.17.1) - 2022-08-17

* SECURITY
  * Correctly escape within tribute.js (#20831) (#20832)
* FEATURES
  * Add support for NuGet API keys (#20721) (#20734)
* ENHANCEMENTS
  * Display project in issue list (#20583)
  * Add disable download source configuration (#20548) (#20579)
* BUGFIXES
  * Use the total issue count for UI (#20785) (#20827)
  * Add proxy host into allow list (#20798) (#20819)
  * Add missing translation for queue flush workers (#20791) (#20792)
  * Improve comment header for mobile (#20781) (#20789)
  * Fix git.Init for doctor sub-command (#20782) (#20783)
  * Check webhooks slice length before calling xorm (#20642) (#20768)
  * Remove manual rollback for failed generated repositories (#20639) (#20762)
  * Use correct field name in npm template (#20675) (#20760)
  * Keep download count on Container tag overwrite (#20728) (#20735)
  * Fix v220 migration to be compatible for MSSQL 2008 r2 (#20702) (#20707)
  * Use request timeout for git service rpc (#20689) (#20693)
  * Send correct NuGet status codes (#20647) (#20677)
  * Use correct context to get package content (#20673) (#20676)
  * Fix the JS error "EventSource is not defined" caused by some non-standard browsers (#20584) (#20663)
  * Add default commit messages to PR for squash merge (#20618) (#20645)
  * Fix package upload for files >32mb (#20622) (#20635)
  * Fix the new-line copy-paste for rendered code (#20612)
  * Clean up and fix clone button script (#20415 & #20600) (#20599)
  *  Fix default merge style (#20564) (#20565)
  * Add repository condition for issue count (#20454) (#20496)
* MISC
  * Make branch icon stand out more (#20726) (#20774)
  * Fix loading button with invalid form (#20754) (#20759)
  * Add username check to doctor (#20140) (#20671)
  * Enable Wire 2 for Internal SSH Server (#20616) (#20617)
  *  Fix SecToTime edge-cases (#20610) (#20611)

Signed-off-by: Andrew Thornton <art27@cantab.net>

* Apply suggestions from code review

Co-authored-by: John Olheiser <john+github@jolheiser.com>

* Update CHANGELOG.md

Co-authored-by: delvh <dev.lh@web.de>

* Update CHANGELOG.md

* Update CHANGELOG.md

* update changelog

* Update CHANGELOG.md

Co-authored-by: John Olheiser <john+github@jolheiser.com>

Signed-off-by: Andrew Thornton <art27@cantab.net>
Co-authored-by: techknowlogick <matti@mdranta.net>
Co-authored-by: John Olheiser <john+github@jolheiser.com>
Co-authored-by: delvh <dev.lh@web.de>
Co-authored-by: Lauris BH <lauris@nix.lv>
Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>
Co-authored-by: techknowlogick <techknowlogick@gitea.io>
jolheiser added a commit to jolheiser/gitea that referenced this pull request Aug 22, 2022
* Changelog 1.17.1

 ## [1.17.1](https://github.com/go-gitea/gitea/releases/tag/1.17.1) - 2022-08-17

* SECURITY
  * Correctly escape within tribute.js (go-gitea#20831) (go-gitea#20832)
* FEATURES
  * Add support for NuGet API keys (go-gitea#20721) (go-gitea#20734)
* ENHANCEMENTS
  * Display project in issue list (go-gitea#20583)
  * Add disable download source configuration (go-gitea#20548) (go-gitea#20579)
* BUGFIXES
  * Use the total issue count for UI (go-gitea#20785) (go-gitea#20827)
  * Add proxy host into allow list (go-gitea#20798) (go-gitea#20819)
  * Add missing translation for queue flush workers (go-gitea#20791) (go-gitea#20792)
  * Improve comment header for mobile (go-gitea#20781) (go-gitea#20789)
  * Fix git.Init for doctor sub-command (go-gitea#20782) (go-gitea#20783)
  * Check webhooks slice length before calling xorm (go-gitea#20642) (go-gitea#20768)
  * Remove manual rollback for failed generated repositories (go-gitea#20639) (go-gitea#20762)
  * Use correct field name in npm template (go-gitea#20675) (go-gitea#20760)
  * Keep download count on Container tag overwrite (go-gitea#20728) (go-gitea#20735)
  * Fix v220 migration to be compatible for MSSQL 2008 r2 (go-gitea#20702) (go-gitea#20707)
  * Use request timeout for git service rpc (go-gitea#20689) (go-gitea#20693)
  * Send correct NuGet status codes (go-gitea#20647) (go-gitea#20677)
  * Use correct context to get package content (go-gitea#20673) (go-gitea#20676)
  * Fix the JS error "EventSource is not defined" caused by some non-standard browsers (go-gitea#20584) (go-gitea#20663)
  * Add default commit messages to PR for squash merge (go-gitea#20618) (go-gitea#20645)
  * Fix package upload for files >32mb (go-gitea#20622) (go-gitea#20635)
  * Fix the new-line copy-paste for rendered code (go-gitea#20612)
  * Clean up and fix clone button script (go-gitea#20415 & go-gitea#20600) (go-gitea#20599)
  *  Fix default merge style (go-gitea#20564) (go-gitea#20565)
  * Add repository condition for issue count (go-gitea#20454) (go-gitea#20496)
* MISC
  * Make branch icon stand out more (go-gitea#20726) (go-gitea#20774)
  * Fix loading button with invalid form (go-gitea#20754) (go-gitea#20759)
  * Add username check to doctor (go-gitea#20140) (go-gitea#20671)
  * Enable Wire 2 for Internal SSH Server (go-gitea#20616) (go-gitea#20617)
  *  Fix SecToTime edge-cases (go-gitea#20610) (go-gitea#20611)

Signed-off-by: Andrew Thornton <art27@cantab.net>

* Apply suggestions from code review

Co-authored-by: John Olheiser <john+github@jolheiser.com>

* Update CHANGELOG.md

Co-authored-by: delvh <dev.lh@web.de>

* Update CHANGELOG.md

* Update CHANGELOG.md

* update changelog

* Update CHANGELOG.md

Co-authored-by: John Olheiser <john+github@jolheiser.com>

Signed-off-by: Andrew Thornton <art27@cantab.net>
Co-authored-by: techknowlogick <matti@mdranta.net>
Co-authored-by: John Olheiser <john+github@jolheiser.com>
Co-authored-by: delvh <dev.lh@web.de>
Co-authored-by: Lauris BH <lauris@nix.lv>
Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>
Co-authored-by: techknowlogick <techknowlogick@gitea.io>
lunny added a commit that referenced this pull request Aug 23, 2022
* Changelog 1.17.1 (#20833)

* Changelog 1.17.1

 ## [1.17.1](https://github.com/go-gitea/gitea/releases/tag/1.17.1) - 2022-08-17

* SECURITY
  * Correctly escape within tribute.js (#20831) (#20832)
* FEATURES
  * Add support for NuGet API keys (#20721) (#20734)
* ENHANCEMENTS
  * Display project in issue list (#20583)
  * Add disable download source configuration (#20548) (#20579)
* BUGFIXES
  * Use the total issue count for UI (#20785) (#20827)
  * Add proxy host into allow list (#20798) (#20819)
  * Add missing translation for queue flush workers (#20791) (#20792)
  * Improve comment header for mobile (#20781) (#20789)
  * Fix git.Init for doctor sub-command (#20782) (#20783)
  * Check webhooks slice length before calling xorm (#20642) (#20768)
  * Remove manual rollback for failed generated repositories (#20639) (#20762)
  * Use correct field name in npm template (#20675) (#20760)
  * Keep download count on Container tag overwrite (#20728) (#20735)
  * Fix v220 migration to be compatible for MSSQL 2008 r2 (#20702) (#20707)
  * Use request timeout for git service rpc (#20689) (#20693)
  * Send correct NuGet status codes (#20647) (#20677)
  * Use correct context to get package content (#20673) (#20676)
  * Fix the JS error "EventSource is not defined" caused by some non-standard browsers (#20584) (#20663)
  * Add default commit messages to PR for squash merge (#20618) (#20645)
  * Fix package upload for files >32mb (#20622) (#20635)
  * Fix the new-line copy-paste for rendered code (#20612)
  * Clean up and fix clone button script (#20415 & #20600) (#20599)
  *  Fix default merge style (#20564) (#20565)
  * Add repository condition for issue count (#20454) (#20496)
* MISC
  * Make branch icon stand out more (#20726) (#20774)
  * Fix loading button with invalid form (#20754) (#20759)
  * Add username check to doctor (#20140) (#20671)
  * Enable Wire 2 for Internal SSH Server (#20616) (#20617)
  *  Fix SecToTime edge-cases (#20610) (#20611)

Signed-off-by: Andrew Thornton <art27@cantab.net>

* Apply suggestions from code review

Co-authored-by: John Olheiser <john+github@jolheiser.com>

* Update CHANGELOG.md

Co-authored-by: delvh <dev.lh@web.de>

* Update CHANGELOG.md

* Update CHANGELOG.md

* update changelog

* Update CHANGELOG.md

Co-authored-by: John Olheiser <john+github@jolheiser.com>

Signed-off-by: Andrew Thornton <art27@cantab.net>
Co-authored-by: techknowlogick <matti@mdranta.net>
Co-authored-by: John Olheiser <john+github@jolheiser.com>
Co-authored-by: delvh <dev.lh@web.de>
Co-authored-by: Lauris BH <lauris@nix.lv>
Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>
Co-authored-by: techknowlogick <techknowlogick@gitea.io>

* update changelog

Signed-off-by: Andrew Thornton <art27@cantab.net>
Co-authored-by: zeripath <art27@cantab.net>
Co-authored-by: techknowlogick <matti@mdranta.net>
Co-authored-by: delvh <dev.lh@web.de>
Co-authored-by: Lauris BH <lauris@nix.lv>
Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>
Co-authored-by: techknowlogick <techknowlogick@gitea.io>
vsysoev pushed a commit to IntegraSDL/gitea that referenced this pull request Aug 28, 2022
* Changelog 1.17.1 (go-gitea#20833)

* Changelog 1.17.1

 ## [1.17.1](https://github.com/go-gitea/gitea/releases/tag/1.17.1) - 2022-08-17

* SECURITY
  * Correctly escape within tribute.js (go-gitea#20831) (go-gitea#20832)
* FEATURES
  * Add support for NuGet API keys (go-gitea#20721) (go-gitea#20734)
* ENHANCEMENTS
  * Display project in issue list (go-gitea#20583)
  * Add disable download source configuration (go-gitea#20548) (go-gitea#20579)
* BUGFIXES
  * Use the total issue count for UI (go-gitea#20785) (go-gitea#20827)
  * Add proxy host into allow list (go-gitea#20798) (go-gitea#20819)
  * Add missing translation for queue flush workers (go-gitea#20791) (go-gitea#20792)
  * Improve comment header for mobile (go-gitea#20781) (go-gitea#20789)
  * Fix git.Init for doctor sub-command (go-gitea#20782) (go-gitea#20783)
  * Check webhooks slice length before calling xorm (go-gitea#20642) (go-gitea#20768)
  * Remove manual rollback for failed generated repositories (go-gitea#20639) (go-gitea#20762)
  * Use correct field name in npm template (go-gitea#20675) (go-gitea#20760)
  * Keep download count on Container tag overwrite (go-gitea#20728) (go-gitea#20735)
  * Fix v220 migration to be compatible for MSSQL 2008 r2 (go-gitea#20702) (go-gitea#20707)
  * Use request timeout for git service rpc (go-gitea#20689) (go-gitea#20693)
  * Send correct NuGet status codes (go-gitea#20647) (go-gitea#20677)
  * Use correct context to get package content (go-gitea#20673) (go-gitea#20676)
  * Fix the JS error "EventSource is not defined" caused by some non-standard browsers (go-gitea#20584) (go-gitea#20663)
  * Add default commit messages to PR for squash merge (go-gitea#20618) (go-gitea#20645)
  * Fix package upload for files >32mb (go-gitea#20622) (go-gitea#20635)
  * Fix the new-line copy-paste for rendered code (go-gitea#20612)
  * Clean up and fix clone button script (go-gitea#20415 & go-gitea#20600) (go-gitea#20599)
  *  Fix default merge style (go-gitea#20564) (go-gitea#20565)
  * Add repository condition for issue count (go-gitea#20454) (go-gitea#20496)
* MISC
  * Make branch icon stand out more (go-gitea#20726) (go-gitea#20774)
  * Fix loading button with invalid form (go-gitea#20754) (go-gitea#20759)
  * Add username check to doctor (go-gitea#20140) (go-gitea#20671)
  * Enable Wire 2 for Internal SSH Server (go-gitea#20616) (go-gitea#20617)
  *  Fix SecToTime edge-cases (go-gitea#20610) (go-gitea#20611)

Signed-off-by: Andrew Thornton <art27@cantab.net>

* Apply suggestions from code review

Co-authored-by: John Olheiser <john+github@jolheiser.com>

* Update CHANGELOG.md

Co-authored-by: delvh <dev.lh@web.de>

* Update CHANGELOG.md

* Update CHANGELOG.md

* update changelog

* Update CHANGELOG.md

Co-authored-by: John Olheiser <john+github@jolheiser.com>

Signed-off-by: Andrew Thornton <art27@cantab.net>
Co-authored-by: techknowlogick <matti@mdranta.net>
Co-authored-by: John Olheiser <john+github@jolheiser.com>
Co-authored-by: delvh <dev.lh@web.de>
Co-authored-by: Lauris BH <lauris@nix.lv>
Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>
Co-authored-by: techknowlogick <techknowlogick@gitea.io>

* update changelog

Signed-off-by: Andrew Thornton <art27@cantab.net>
Co-authored-by: zeripath <art27@cantab.net>
Co-authored-by: techknowlogick <matti@mdranta.net>
Co-authored-by: delvh <dev.lh@web.de>
Co-authored-by: Lauris BH <lauris@nix.lv>
Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>
Co-authored-by: techknowlogick <techknowlogick@gitea.io>
@go-gitea go-gitea locked and limited conversation to collaborators May 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
backport/done All backports for this PR have been created issue/regression Issue needs no code to be fixed, only a description on how to fix it yourself lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Incorrect handling of durations in SecToTime()
6 participants