Skip to content

WIP: Cleaning Code#481

Merged
mattverse merged 16 commits intomainfrom
mattverse/code-clean
Oct 7, 2021
Merged

WIP: Cleaning Code#481
mattverse merged 16 commits intomainfrom
mattverse/code-clean

Conversation

@mattverse
Copy link
Copy Markdown
Contributor

WIP: Cleaning code!

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Sep 18, 2021

Codecov Report

Merging #481 (6db6295) into main (9f1e0c7) will decrease coverage by 0.01%.
The diff coverage is 70.37%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #481      +/-   ##
==========================================
- Coverage   20.41%   20.40%   -0.02%     
==========================================
  Files         163      163              
  Lines       23206    23209       +3     
==========================================
- Hits         4738     4736       -2     
- Misses      17694    17698       +4     
- Partials      774      775       +1     
Impacted Files Coverage Δ
app/app.go 85.47% <ø> (ø)
x/gamm/keeper/pool_service.go 61.44% <0.00%> (ø)
x/gamm/types/genesis.pb.go 0.90% <ø> (ø)
x/incentives/types/keys.go 0.00% <ø> (ø)
x/pool-incentives/keeper/distr.go 82.82% <ø> (-1.14%) ⬇️
x/pool-incentives/types/genesis.go 32.00% <0.00%> (-2.79%) ⬇️
x/claim/keeper/claim.go 64.16% <33.33%> (-1.09%) ⬇️
x/claim/genesis.go 60.00% <50.00%> (-9.24%) ⬇️
x/gamm/keeper/pool.go 81.08% <50.00%> (+1.35%) ⬆️
x/gamm/genesis.go 64.28% <100.00%> (ø)
... and 8 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 9f1e0c7...6db6295. Read the comment docs.

@ValarDragon
Copy link
Copy Markdown
Member

LGTM other than claims parameter and simulation log line!

Copy link
Copy Markdown
Member

@ValarDragon ValarDragon left a comment

Choose a reason for hiding this comment

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

LGTM now. Made a commit with the remaining deletes we talked about. Can you merge after having a look?

@mattverse mattverse merged commit ea6bda7 into main Oct 7, 2021
@mattverse mattverse deleted the mattverse/code-clean branch October 7, 2021 05:50
@mattverse mattverse restored the mattverse/code-clean branch October 7, 2021 08:52
@mattverse
Copy link
Copy Markdown
Contributor Author

Srry my bad, thought I did a final check on everything to confirm they are all good :(

return err
}

if data.DistrInfo.TotalWeight.LT(sdk.NewInt(0)) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

default DistrInfo value is nil, this throw an error:

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x1358cdc]

goroutine 1 [running]:
github.com/osmosis-labs/osmosis/x/pool-incentives/types.ValidateGenesis(0xc00048a4b0)
        github.com/osmosis-labs/osmosis/x/pool-incentives/types/genesis.go:53 +0x7c

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

4 participants