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

Move forceprune logic into functions, make db open/closes all in a single scope #1321

Merged
merged 4 commits into from
Apr 22, 2022

Conversation

ValarDragon
Copy link
Member

@ValarDragon ValarDragon commented Apr 22, 2022

What is the purpose of the change

Follow up PR to #1262, it shuffles the logic into functions, so each db opening and closing all happens in a single scope. (And therefore defer closes work as expected)

Beyond DB closing behavior, the logic should be identical

cc @moniya12

Brief change log

No logic change, just code structure updates to the forceprune command. Also updates variables names from snake case to camel case (matching the rest of the repo)

Testing and Verifying

This change is a trivial rework / code cleanup without any test coverage.

Documentation and Release Note

  • Does this pull request introduce a new feature or user-facing behavior changes? no
  • Is a relevant changelog entry added to the Unreleased section in CHANGELOG.md? no (not needed)

@ValarDragon ValarDragon requested review from czarcas7ic and p0mvn April 22, 2022 04:25
Comment on lines +136 to +138
opts := opt.Options{
DisableSeeksCompaction: true,
}
Copy link
Member Author

Choose a reason for hiding this comment

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

I chose to duplicate this in each function, since its pretty unclear to me that longer term, these won't be different for every function

@ValarDragon ValarDragon added the A:backport/v7.x Do not use. backport patches to v7.x branch label Apr 22, 2022
@codecov-commenter
Copy link

codecov-commenter commented Apr 22, 2022

Codecov Report

Merging #1321 (5066805) into main (84f0194) will decrease coverage by 0.39%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main    #1321      +/-   ##
==========================================
- Coverage   19.81%   19.42%   -0.40%     
==========================================
  Files         210      199      -11     
  Lines       27884    27455     -429     
==========================================
- Hits         5525     5332     -193     
+ Misses      21333    21133     -200     
+ Partials     1026      990      -36     
Impacted Files Coverage Δ
x/claim/keeper/grpc_query.go
x/claim/handler.go
x/claim/client/cli/tx.go
x/claim/abci.go
x/claim/module.go
x/claim/keeper/params.go
x/claim/genesis.go
x/claim/keeper/hooks.go
x/claim/client/cli/query.go
x/claim/keeper/keeper.go
... and 1 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 15f131e...5066805. Read the comment docs.

@ValarDragon ValarDragon merged commit 039f686 into main Apr 22, 2022
@ValarDragon ValarDragon deleted the dev/shuffle_forceprune_logic branch April 22, 2022 05:05
mergify bot pushed a commit that referenced this pull request Apr 22, 2022
…ngle scope (#1321)

* Move forceprune logic into functions, make db open/closes all in a single scope

* switch remaining snake cases to camel case

* Add changelog entry for original valnodes PR

* Tune changelog message

(cherry picked from commit 039f686)

# Conflicts:
#	CHANGELOG.md
ValarDragon pushed a commit that referenced this pull request Apr 22, 2022
…ngle scope (backport #1321) (#1323)

* Move forceprune logic into functions, make db open/closes all in a single scope (#1321)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A:backport/v7.x Do not use. backport patches to v7.x branch
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants