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

refactor: Remove storev2alpha1 #13371

Merged
merged 9 commits into from
Sep 23, 2022
Merged

Conversation

faddat
Copy link
Contributor

@faddat faddat commented Sep 23, 2022

Description

This code seems to be unused and we'll have less work to do with it removed.

NB: if I am wrong about this, do not merge


Author Checklist

All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.

I have...

  • included the correct type prefix in the PR title
  • added ! to the type prefix if API or client breaking change
  • targeted the correct branch (see PR Targeting)
  • provided a link to the relevant issue or specification
  • followed the guidelines for building modules
  • included the necessary unit and integration tests
  • added a changelog entry to CHANGELOG.md
  • included comments for documenting Go code
  • updated the relevant documentation or specification
  • reviewed "Files changed" and left comments if necessary
  • confirmed all CI checks have passed

Reviewers Checklist

All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.

I have...

  • confirmed the correct type prefix in the PR title
  • confirmed ! in the type prefix if API or client breaking change
  • confirmed all author checklist items have been addressed
  • reviewed state machine logic
  • reviewed API design and naming
  • reviewed documentation is accurate
  • reviewed tests and test coverage
  • manually tested (if applicable)

@faddat faddat requested a review from a team as a code owner September 23, 2022 12:33
@faddat faddat changed the title !feat: Remove storev2alpha1 refactor: Remove storev2alpha1 Sep 23, 2022
@codecov
Copy link

codecov bot commented Sep 23, 2022

Codecov Report

Merging #13371 (9256568) into main (4fe7797) will decrease coverage by 1.57%.
The diff coverage is 28.00%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main   #13371      +/-   ##
==========================================
- Coverage   55.87%   54.30%   -1.58%     
==========================================
  Files         646      639       -7     
  Lines       54895    54585     -310     
==========================================
- Hits        30675    29644    -1031     
- Misses      21762    22561     +799     
+ Partials     2458     2380      -78     
Impacted Files Coverage Δ
baseapp/grpcrouter.go 90.00% <ø> (ø)
baseapp/grpcrouter_helpers.go 25.00% <ø> (ø)
baseapp/grpcserver.go 1.72% <ø> (ø)
baseapp/msg_service_router.go 85.29% <ø> (+4.41%) ⬆️
baseapp/options.go 67.92% <0.00%> (-0.60%) ⬇️
client/broadcast.go 54.54% <ø> (+2.99%) ⬆️
client/cmd.go 57.73% <ø> (ø)
client/config/toml.go 55.55% <ø> (ø)
client/context.go 54.49% <ø> (-1.79%) ⬇️
client/flags/flags.go 19.35% <ø> (-0.32%) ⬇️
... and 247 more

@julienrbrt
Copy link
Member

AFAIK, there are discussions of moving it to a feature branch, so this PR might be needed.

@faddat
Copy link
Contributor Author

faddat commented Sep 23, 2022

Would make it easier to complete work on iavl and cosmos-db integration

@faddat
Copy link
Contributor Author

faddat commented Sep 23, 2022

@julienrbrt -- any ideas about this e2e test?

@tac0turtle
Copy link
Member

AFAIK, there are discussions of moving it to a feature branch, so this PR might be needed.

https://github.com/cosmos/cosmos-sdk/tree/store/v2 features branch is already live

@faddat
Copy link
Contributor Author

faddat commented Sep 23, 2022

Landing in Dallas-- hope to meet you both soon!

❤️

@faddat
Copy link
Contributor Author

faddat commented Sep 23, 2022

I'll fix the changelog entry for the other one, here. Sorry bout that!

CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
@julienrbrt
Copy link
Member

julienrbrt commented Sep 23, 2022

@julienrbrt -- any ideas about this e2e test?

That test iirc is flaky. Retrying should suffice.

Due to #13370, you'll want to remove these two lines: I had to do it before, so it isn't needed anymore.

@faddat
Copy link
Contributor Author

faddat commented Sep 23, 2022

Cool! I'll take care of it as soon as I'm waiting for next flight.

@faddat
Copy link
Contributor Author

faddat commented Sep 23, 2022

dallas -> miami

@julienrbrt julienrbrt enabled auto-merge (squash) September 23, 2022 19:05
@julienrbrt julienrbrt merged commit b10097f into cosmos:main Sep 23, 2022
Wryhder pushed a commit to Wryhder/cosmos-sdk that referenced this pull request Oct 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants