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.

Sign up for GitHub

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

Jump to bottom

Fix close issue but time watcher still running #17643

Merged
merged 6 commits into from
Nov 21, 2021
Merged

Fix close issue but time watcher still running #17643

merged 6 commits into from
Nov 21, 2021

Conversation

lunny
Copy link
Member

@lunny lunny commented Nov 15, 2021

Fix #11374

@lunny lunny added this to the 1.16.0 milestone Nov 15, 2021
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Nov 15, 2021
@lunny lunny force-pushed the lunny/fix_issue_close_bug branch 4 times, most recently from 88a6efc to aad2199 Compare November 20, 2021 13:34
@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 Nov 21, 2021
Gusted
Gusted reviewed Nov 21, 2021
View reviewed changes
models/issue_stopwatch.go Outdated Show resolved Hide resolved
models/issue_stopwatch.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 Nov 21, 2021
@codecov-commenter
Copy link

Codecov Report

Merging #17643 (35cc99c) into main (a85e75b) will increase coverage by 0.06%.
The diff coverage is 58.56%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main   #17643      +/-   ##
==========================================
+ Coverage   45.44%   45.51%   +0.06%     
==========================================
  Files         802      807       +5     
  Lines       89233    89842     +609     
==========================================
+ Hits        40556    40892     +336     
- Misses      42140    42408     +268     
- Partials     6537     6542       +5
Impacted Files Coverage Δ
cmd/migrate_storage.go 0.00% <0.00%> (ø)
cmd/web_graceful.go 0.00% <ø> (ø)
cmd/web_https.go 0.00% <0.00%> (ø)
cmd/web_letsencrypt.go 0.00% <0.00%> (ø)
models/error.go 38.76% <0.00%> (-0.06%) ⬇️
models/issue_list.go 71.80% <0.00%> (ø)
models/issue_lock.go 0.00% <0.00%> (ø)
models/statistic.go 0.00% <0.00%> (ø)
models/user.go 56.63% <ø> (-0.06%) ⬇️
modules/doctor/dbconsistency.go 6.83% <0.00%> (ø)
... and 120 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ab09296...35cc99c. Read the comment docs.

@lunny lunny merged commit 0add627 into go-gitea:main Nov 21, 2021
@lunny lunny deleted the lunny/fix_issue_close_bug branch November 21, 2021 09:11
@lunny lunny mentioned this pull request Nov 22, 2021
Merged
@lunny lunny added the backport/done All backports for this PR have been created label Nov 22, 2021
@zeripath zeripath mentioned this pull request Jan 15, 2022
Closed
@zeripath
Copy link
Contributor

I think this is incorrect and rather the error handling should be happening at the caller end.

@zeripath zeripath mentioned this pull request Jan 15, 2022
Closed
zeripath added a commit to zeripath/gitea that referenced this pull request Jan 18, 2022
@zeripath
Restore propagation of ErrDependenciesLeft
f67b5ee 
Unfortunately go-gitea#17643 prevented all propagation of ErrDependenciesLeft meaning
that dependency errors that prevent closing of issues get swallowed.

This PR restores propagation of the error but instead swallows the error in the
places where it needs to be swallowed.

Fix go-gitea#18223

Signed-off-by: Andrew Thornton <art27@cantab.net>
@zeripath zeripath mentioned this pull request Jan 18, 2022
Merged
6543 pushed a commit that referenced this pull request Jan 18, 2022
@zeripath
Restore propagation of ErrDependenciesLeft (#18325)
4a20ead 
Unfortunately #17643 prevented all propagation of ErrDependenciesLeft meaning
that dependency errors that prevent closing of issues get swallowed.

This PR restores propagation of the error but instead swallows the error in the
places where it needs to be swallowed.

Fix #18223

Signed-off-by: Andrew Thornton <art27@cantab.net>
Chianina pushed a commit to Chianina/gitea that referenced this pull request Mar 28, 2022
@lunny
Fix close issue but time watcher still running (go-gitea#17643)
5edbb08 
* Fix close issue but time watcher still running

* refactor stopwatch codes

* Fix test

* Fix test

* Fix typo

* Fix test
Chianina pushed a commit to Chianina/gitea that referenced this pull request Mar 28, 2022
@zeripath
Restore propagation of ErrDependenciesLeft (go-gitea#18325)
c45a7f3 
Unfortunately go-gitea#17643 prevented all propagation of ErrDependenciesLeft meaning
that dependency errors that prevent closing of issues get swallowed.

This PR restores propagation of the error but instead swallows the error in the
places where it needs to be swallowed.

Fix go-gitea#18223

Signed-off-by: Andrew Thornton <art27@cantab.net>
@go-gitea go-gitea locked and limited conversation to collaborators Apr 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@wxiaoguang wxiaoguang wxiaoguang approved these changes

@Gusted Gusted Gusted approved these changes

Assignees
No one assigned
Labels
backport/done All backports for this PR have been created lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. type/bug
Projects
None yet
Milestone
1.16.0
Development

Successfully merging this pull request may close these issues.

The timetracker will not be stop from a issue reference in the message of a PR
6 participants
@lunny @codecov-commenter @zeripath @wxiaoguang @Gusted @GiteaBot