Skip to content

Conversation

zeripath
Copy link
Contributor

A recent bug occurred due to a gotcha in the settings code for queues. This code used to do an unnecessary struct to map[string]interface{} conversion that with a small amount renaming is removable.

There a few changes in the helper.go:toConfig() function which allow for more options in future.

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

…problems

Signed-off-by: Andrew Thornton <art27@cantab.net>
@zeripath zeripath added the type/refactoring Existing code has been cleaned up. There should be no new functionality. label Sep 29, 2020
@codecov-commenter
Copy link

Codecov Report

Merging #12976 into master will decrease coverage by 0.03%.
The diff coverage is 21.05%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #12976      +/-   ##
==========================================
- Coverage   42.60%   42.56%   -0.04%     
==========================================
  Files         671      671              
  Lines       73627    73620       -7     
==========================================
- Hits        31368    31338      -30     
- Misses      37179    37194      +15     
- Partials     5080     5088       +8     
Impacted Files Coverage Δ
modules/queue/helper.go 47.36% <0.00%> (-14.71%) ⬇️
modules/queue/setting.go 31.25% <25.00%> (-14.43%) ⬇️
modules/setting/queue.go 76.41% <100.00%> (+0.22%) ⬆️
modules/indexer/stats/db.go 60.86% <0.00%> (-8.70%) ⬇️
modules/charset/charset.go 68.53% <0.00%> (-2.25%) ⬇️
services/gitdiff/gitdiff.go 72.85% <0.00%> (-1.64%) ⬇️
modules/queue/unique_queue_disk_channel.go 53.84% <0.00%> (-1.54%) ⬇️
services/pull/pull.go 40.78% <0.00%> (-0.50%) ⬇️
models/unit.go 49.31% <0.00%> (+2.73%) ⬆️

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 3878e98...2421bdc. Read the comment docs.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Sep 29, 2020
Copy link
Member

@6543 6543 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting code comments - like them :)

@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 Sep 29, 2020
@zeripath zeripath added this to the 1.14.0 milestone Oct 1, 2020
@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 Oct 15, 2020
@codecov-io
Copy link

Codecov Report

Merging #12976 into master will increase coverage by 0.01%.
The diff coverage is 21.05%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #12976      +/-   ##
==========================================
+ Coverage   42.05%   42.07%   +0.01%     
==========================================
  Files         681      681              
  Lines       75121    75114       -7     
==========================================
+ Hits        31594    31606      +12     
+ Misses      38372    38354      -18     
+ Partials     5155     5154       -1     
Impacted Files Coverage Δ
modules/queue/helper.go 47.36% <0.00%> (-14.71%) ⬇️
modules/queue/setting.go 31.25% <25.00%> (-14.43%) ⬇️
modules/setting/queue.go 67.92% <100.00%> (+0.30%) ⬆️
modules/process/manager.go 72.50% <0.00%> (-2.50%) ⬇️
services/pull/pull.go 40.78% <0.00%> (ø)
models/issue_comment.go 53.08% <0.00%> (+0.15%) ⬆️
models/error.go 34.39% <0.00%> (+0.50%) ⬆️
modules/git/repo.go 46.70% <0.00%> (+0.50%) ⬆️
models/gpg_key.go 53.90% <0.00%> (+0.57%) ⬆️
modules/log/event.go 60.84% <0.00%> (+0.94%) ⬆️
... and 6 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 e374bb7...413ecbc. Read the comment docs.

@techknowlogick techknowlogick merged commit c8f7a6b into go-gitea:master Oct 15, 2020
@zeripath zeripath deleted the simplify-queue-settings branch October 15, 2020 22:02
ivanvc added a commit to ivanvc/gitea that referenced this pull request Oct 16, 2020
…ments-in-pull-request-label-style

* origin/master:
  [skip ci] Updated translations via Crowdin
  Fix diff skipping lines (go-gitea#13154)
  Update go-version v1.2.3 -> v1.2.4 (go-gitea#13169)
  Vendor Update Go Libs (go-gitea#13166)
  Prevent panics with missing storage (go-gitea#13164)
  Improve users management through the CLI (go-gitea#6001) (go-gitea#10492)
  Change order of possible-owner organizations to alphabetical (go-gitea#13160)
  Slightly simplify the queue settings code to help reduce the risk of problems (go-gitea#12976)
  [Vendor] Update go-ldap to v3.2.4 (go-gitea#13163)
  [skip ci] Updated translations via Crowdin
  Update external-renderers.en-us.md (go-gitea#13165)
@go-gitea go-gitea locked and limited conversation to collaborators Nov 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. type/refactoring Existing code has been cleaned up. There should be no new functionality.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants