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

Remove dependency of types/module package on x/simulation #5835

Merged
merged 28 commits into from
Mar 23, 2020

Conversation

jgimeno
Copy link
Contributor

@jgimeno jgimeno commented Mar 19, 2020

Closes: #5724

Description


For contributor use:

  • Targeted PR against correct branch (see CONTRIBUTING.md)
  • Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
  • Code follows the module structure standards.
  • Wrote unit and integration tests
  • Updated relevant documentation (docs/) or specification (x/<module>/spec/)
  • Added relevant godoc comments.
  • Added a relevant changelog entry to the Unreleased section in CHANGELOG.md
  • Re-reviewed Files changed in the Github PR explorer

For admin use:

  • Added appropriate labels to PR (ex. WIP, R4R, docs, etc)
  • Reviewers assigned
  • Squashed all commits, uses message "Merge pull request #XYZ: [title]" (coding standards)

@jgimeno jgimeno self-assigned this Mar 19, 2020
x/auth/simulation/params.go Outdated Show resolved Hide resolved
simapp/helpers/test_helpers.go Outdated Show resolved Hide resolved
x/simulation/operation.go Outdated Show resolved Hide resolved
x/simulation/params.go Outdated Show resolved Hide resolved
x/simulation/simulate.go Outdated Show resolved Hide resolved
x/params/module.go Outdated Show resolved Hide resolved
x/mint/simulation/params.go Outdated Show resolved Hide resolved
@jgimeno
Copy link
Contributor Author

jgimeno commented Mar 20, 2020

@alessio, @fedekunze can you take a look if you see something weird? The dependency is removed now and compiles.

@codecov
Copy link

codecov bot commented Mar 20, 2020

Codecov Report

Merging #5835 into master will not change coverage.
The diff coverage is 0%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #5835   +/-   ##
=======================================
  Coverage   32.32%   32.32%           
=======================================
  Files         353      353           
  Lines       39465    39465           
=======================================
  Hits        12759    12759           
  Misses      25434    25434           
  Partials     1272     1272
Impacted Files Coverage Δ
types/module/module.go 96.15% <ø> (ø) ⬆️
types/module/simulation.go 0% <ø> (ø) ⬆️
x/gov/types/content.go 100% <ø> (ø) ⬆️
x/auth/module.go 50% <0%> (ø) ⬆️
x/bank/module.go 46% <0%> (ø) ⬆️
x/distribution/module.go 54.09% <0%> (ø) ⬆️
x/mint/module.go 49.01% <0%> (ø) ⬆️
x/slashing/module.go 55% <0%> (ø) ⬆️
simapp/utils.go 22.03% <0%> (ø) ⬆️
x/staking/module.go 50.76% <0%> (ø) ⬆️
... and 2 more

@jgimeno jgimeno marked this pull request as ready for review March 20, 2020 17:14
@codecov
Copy link

codecov bot commented Mar 20, 2020

Codecov Report

Merging #5835 into master will not change coverage.
The diff coverage is 5.4%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #5835   +/-   ##
=======================================
  Coverage   32.32%   32.32%           
=======================================
  Files         353      353           
  Lines       39476    39476           
=======================================
  Hits        12760    12760           
  Misses      25440    25440           
  Partials     1276     1276
Impacted Files Coverage Δ
types/module/simulation.go 0% <ø> (ø) ⬆️
types/module/module.go 96.15% <ø> (ø) ⬆️
x/gov/types/content.go 100% <ø> (ø) ⬆️
simapp/config.go 54.28% <ø> (ø) ⬆️
x/auth/module.go 50% <0%> (ø) ⬆️
x/bank/module.go 46% <0%> (ø) ⬆️
x/distribution/module.go 54.09% <0%> (ø) ⬆️
x/mint/module.go 49.01% <0%> (ø) ⬆️
x/slashing/module.go 55% <0%> (ø) ⬆️
x/staking/module.go 50.76% <0%> (ø) ⬆️
... and 3 more

Copy link
Contributor

@alessio alessio left a comment

Choose a reason for hiding this comment

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

ACK. Pleae run make format.

Copy link
Contributor

@alessio alessio left a comment

Choose a reason for hiding this comment

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

Not all deps have been removed

simapp/state.go Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
Co-Authored-By: Alessio Treglia <[email protected]>
@alessio alessio requested a review from tac0turtle March 23, 2020 10:56
x/staking/module.go Outdated Show resolved Hide resolved
@@ -7,9 +7,10 @@ import (
"fmt"
"math/rand"

"github.com/cosmos/cosmos-sdk/x/simulation"
Copy link
Contributor

Choose a reason for hiding this comment

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

File is not goimports-ed (from goimports)

Suggested change
"github.com/cosmos/cosmos-sdk/x/simulation"

@@ -13,12 +13,12 @@ import (
cfg "github.com/tendermint/tendermint/config"
"github.com/tendermint/tendermint/crypto"

simtypes "github.com/cosmos/cosmos-sdk/types/simulation"
Copy link
Contributor

Choose a reason for hiding this comment

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

File is not goimports-ed (from goimports)

Suggested change
simtypes "github.com/cosmos/cosmos-sdk/types/simulation"

@@ -7,6 +7,7 @@ import (
"math/rand"

"github.com/cosmos/cosmos-sdk/x/simulation"
simtypes "github.com/cosmos/cosmos-sdk/types/simulation"
Copy link
Contributor

Choose a reason for hiding this comment

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

File is not gofmt-ed with -s (from gofmt)

Suggested change
simtypes "github.com/cosmos/cosmos-sdk/types/simulation"
simtypes "github.com/cosmos/cosmos-sdk/types/simulation"
"github.com/cosmos/cosmos-sdk/x/simulation"

Copy link
Member

@tac0turtle tac0turtle left a comment

Choose a reason for hiding this comment

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

LGTM 🇪🇸 🐧

@alessio alessio merged commit 49102b1 into master Mar 23, 2020
@alessio alessio deleted the jonathan/5724-module-simulation-top-level-dependency branch March 23, 2020 11:55
larry0x pushed a commit to larry0x/cosmos-sdk that referenced this pull request May 22, 2023
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.

types/module package depends on x/simulation
4 participants