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

lint: Fix lint errors in config/ #5416

Merged
merged 5 commits into from May 24, 2023

Conversation

algochoi
Copy link
Contributor

@algochoi algochoi commented May 24, 2023

Summary

Running golangci-lint run config/ (removing the commit hash gate in golangci.yml) generates the following lint error:

config/consensus.go:682:12: S1033: unnecessary guard around call to delete (gosimple)
                                } else if _, has := cParam.ApprovedUpgrades[consensusVersion]; has {
config/migrate.go:131:5: SA4003: no value of type uint32 is less than 0 (staticcheck)
        if version < 0 {

This PR fixes this lint error by removing the guard around the delete call (the code is functionally the same, since deleting nil map key/value is a no op) and removing the negative check for uint values.

Test Plan

winder
winder previously approved these changes May 24, 2023
@algochoi algochoi changed the title Remove guard around delete to fix lint warning lint: Fix lint errors in config/ May 24, 2023
@algochoi algochoi added the tech debt Things that need re-work for simplification / sanitization to reduce implementation overhead label May 24, 2023
@codecov
Copy link

codecov bot commented May 24, 2023

Codecov Report

Merging #5416 (91c8fb4) into master (1abbd50) will increase coverage by 0.00%.
The diff coverage is 0.00%.

@@           Coverage Diff           @@
##           master    #5416   +/-   ##
=======================================
  Coverage   55.40%   55.40%           
=======================================
  Files         452      452           
  Lines       63855    63853    -2     
=======================================
- Hits        35379    35378    -1     
- Misses      26044    26046    +2     
+ Partials     2432     2429    -3     
Impacted Files Coverage Δ
config/consensus.go 86.19% <0.00%> (ø)
config/migrate.go 78.46% <ø> (+1.18%) ⬆️

... and 7 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@algochoi algochoi added Enhancement and removed tech debt Things that need re-work for simplification / sanitization to reduce implementation overhead labels May 24, 2023
@algochoi algochoi marked this pull request as ready for review May 24, 2023 17:25
Copy link
Contributor

@tzaffi tzaffi left a comment

Choose a reason for hiding this comment

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

LGTM

@winder winder merged commit 44f4da8 into algorand:master May 24, 2023
24 checks passed
@algochoi algochoi deleted the lint-consensus-config branch May 24, 2023 20:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants