Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Move codec.RegisterCrypto and codec.Cdc to new packages #6330

Merged
merged 15 commits into from
Jun 4, 2020

Conversation

aaronc
Copy link
Member

@aaronc aaronc commented Jun 2, 2020

Description

In order to register the multisig type that @marbar3778 added in #6241 with amino (which is necessary for completing #6216), we need to move the global amino Cdc instance outside of the codec package to avoid cyclic dependencies.

This PR fixes this by:

  • moving codec.RegisterCrypto to crypto/codec and having it reference the amino registration in crypto/types/multisig which was added in multisig: add type and bitarray #6241
  • moving codec.Cdc to a new codec/legacy_global package (which has an intentionally ugly name) and deprecating it

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

@codecov
Copy link

codecov bot commented Jun 2, 2020

Codecov Report

Merging #6330 into master will decrease coverage by 0.03%.
The diff coverage is 80.95%.

@@            Coverage Diff             @@
##           master    #6330      +/-   ##
==========================================
- Coverage   55.66%   55.63%   -0.04%     
==========================================
  Files         448      448              
  Lines       26956    26951       -5     
==========================================
- Hits        15005    14994      -11     
- Misses      10877    10883       +6     
  Partials     1074     1074              

@aaronc aaronc changed the title Move codec.Cdc to legacy_global package Move codec.RegisterCrypto and codec.Cdc to new packages Jun 2, 2020
@aaronc aaronc marked this pull request as ready for review June 2, 2020 23:49
@aaronc aaronc added T: API Breaking Breaking changes that impact APIs and the SDK only (not state machine). R4R labels Jun 2, 2020
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

@aaronc aaronc added the A:automerge Automatically merge PR once all prerequisites pass. label Jun 3, 2020
@mergify mergify bot merged commit 81d647e into master Jun 4, 2020
@mergify mergify bot deleted the aaronc/6213-legacy-global branch June 4, 2020 10:38
@clevinson clevinson added this to the v0.39 milestone Jun 11, 2020
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
A:automerge Automatically merge PR once all prerequisites pass. T: API Breaking Breaking changes that impact APIs and the SDK only (not state machine).
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants