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.

Sign up for GitHub

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

Jump to bottom

x/staking: skip HistoricalInfo in simulations #5949

Merged
merged 13 commits into from
Apr 7, 2020
Merged

x/staking: skip HistoricalInfo in simulations #5949

merged 13 commits into from
Apr 7, 2020

Conversation

fedekunze
Copy link
Collaborator

@fedekunze fedekunze commented Apr 7, 2020 •
edited by alexanderbez
Loading

Closes: #XXX

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)

@codecov
Copy link

codecov bot commented Apr 7, 2020 •
edited
Loading

Codecov Report

Merging #5949 into master will increase coverage by 0.02%.
The diff coverage is 91.30%.

@@            Coverage Diff             @@
##           master    #5949      +/-   ##
==========================================
+ Coverage   56.48%   56.50%   +0.02%     
==========================================
  Files         342      342              
  Lines       20270    20278       +8     
==========================================
+ Hits        11449    11459      +10     
+ Misses       7974     7971       -3     
- Partials      847      848       +1

@fedekunze fedekunze marked this pull request as ready for review April 7, 2020 18:18
alexanderbez
alexanderbez approved these changes Apr 7, 2020
View reviewed changes
Copy link
Contributor

@alexanderbez alexanderbez left a comment

Choose a reason for hiding this comment

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

LGTM 👍

x/staking/genesis.go Outdated Show resolved Hide resolved
@fedekunze fedekunze added the A:automerge Automatically merge PR once all prerequisites pass. label Apr 7, 2020
@fedekunze fedekunze removed the A:automerge Automatically merge PR once all prerequisites pass. label Apr 7, 2020
@fedekunze
Copy link
Collaborator Author

waiting for @AdityaSripal's input before merging

@fedekunze fedekunze added the S:blocked Status: Blocked label Apr 7, 2020
@fedekunze fedekunze changed the title x/staking: import and export HistoricalInfo x/staking: skip HistoricalInfo in simulations Apr 7, 2020
@fedekunze fedekunze added WIP and removed R4R labels Apr 7, 2020
fedekunze
fedekunze commented Apr 7, 2020
View reviewed changes
.github/workflows/sims.yml
run: |
make test-sim-import-export
Copy link
Collaborator Author

@fedekunze fedekunze Apr 7, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

test-sim-after-import workflow was running make test-sim-import-export

@fedekunze fedekunze added R4R and removed WIP S:blocked Status: Blocked labels Apr 7, 2020
AdityaSripal
AdityaSripal approved these changes Apr 7, 2020
View reviewed changes
Copy link
Member

@AdityaSripal AdityaSripal left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@fedekunze fedekunze added the A:automerge Automatically merge PR once all prerequisites pass. label Apr 7, 2020
@mergify mergify bot merged commit 7325692 into master Apr 7, 2020
@mergify mergify bot deleted the export-hist-info branch April 7, 2020 22:33
@fedekunze fedekunze mentioned this pull request Apr 7, 2020
Closed
jackzampolin
jackzampolin reviewed Apr 8, 2020
View reviewed changes
Copy link
Member

@jackzampolin jackzampolin left a comment

Choose a reason for hiding this comment

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

This is awesome and thank you @fedekunze 🙏

larry0x pushed a commit to larry0x/cosmos-sdk that referenced this pull request May 22, 2023
@fedekunze
x/staking: skip HistoricalInfo in simulations (cosmos#5949)
fb01747 
* x/staking: import and export HistoricalInfo

* staking/types: add HistoricalInfo to GenesisState

* changelog

* add staking module to app BeginBlockers

* remove JSON files

* address comments from review

* add HistoricalInfoKey to skipped prefixes

* fix DiffKVStores

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

@jackzampolin jackzampolin jackzampolin left review comments

@alexanderbez alexanderbez alexanderbez approved these changes

@AdityaSripal AdityaSripal AdityaSripal approved these changes

@alessio alessio Awaiting requested review from alessio

@hschoenburg hschoenburg Awaiting requested review from hschoenburg

@nylira nylira Awaiting requested review from nylira

@cwgoes cwgoes Awaiting requested review from cwgoes

@jgimeno jgimeno Awaiting requested review from jgimeno

Assignees
No one assigned
Labels
A:automerge Automatically merge PR once all prerequisites pass. C:x/staking T:Bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@fedekunze @alexanderbez @jackzampolin @AdityaSripal