Skip to content

crypto: Remove unnecessary prefixes from amino route variable names#2205

Merged
ebuchman merged 5 commits intodevelopfrom
dev/fix_crypto_prefixing
Aug 14, 2018
Merged

crypto: Remove unnecessary prefixes from amino route variable names#2205
ebuchman merged 5 commits intodevelopfrom
dev/fix_crypto_prefixing

Conversation

@ValarDragon
Copy link
Contributor

@ValarDragon ValarDragon commented Aug 10, 2018

REBASE ONTO DEVELOP ONCE #2164 IS MERGED

  • Updated CHANGELOG_PENDING.md - should this be updated? I don't think anyone was using these variables.

Copy link
Contributor

@melekes melekes left a comment

Choose a reason for hiding this comment

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

🥇 🏎 🍰

@melekes
Copy link
Contributor

melekes commented Aug 14, 2018

Updated CHANGELOG_PENDING.md - should this be updated? I don't think anyone was using these variables.

you never know. let's add an entry

@ebuchman
Copy link
Contributor

Should also change PubkeyAminoRoute to tendermint/PubkeyMultisigThreshold (ie. multisig first, since then we'll also have PubKeyMultisigWeighted)

@ebuchman
Copy link
Contributor

I got this

@ebuchman ebuchman force-pushed the dev/fix_crypto_prefixing branch from 9ac0291 to fa315fa Compare August 14, 2018 21:57
@ebuchman
Copy link
Contributor

Should we do the same thing for eg. ed25519.PubKeyEd25519 -> ed25519.PubKey ?

@ValarDragon
Copy link
Contributor Author

ValarDragon commented Aug 14, 2018

I was considering that. The reason I didn't change it is that it may make some amino debugging harder. AFAICT, amino prints the struct name, not the path to it. (So you wouldn't see the ed25519) Upon further thought though, this probably isn't a hard change to make in amino

@codecov-io
Copy link

codecov-io commented Aug 14, 2018

Codecov Report

Merging #2205 into develop will decrease coverage by 0.09%.
The diff coverage is 87.5%.

@@            Coverage Diff             @@
##           develop    #2205     +/-   ##
==========================================
- Coverage    62.43%   62.33%   -0.1%     
==========================================
  Files          215      215             
  Lines        17627    17655     +28     
==========================================
  Hits         11005    11005             
- Misses        5711     5735     +24     
- Partials       911      915      +4
Impacted Files Coverage Δ
crypto/multisig/wire.go 100% <100%> (ø) ⬆️
crypto/secp256k1/secp256k1.go 57.37% <100%> (ø) ⬆️
crypto/ed25519/ed25519.go 43.58% <75%> (ø) ⬆️
consensus/reactor.go 73.02% <0%> (-0.81%) ⬇️
consensus/state.go 76.09% <0%> (-0.49%) ⬇️
consensus/wal_generator.go 80.23% <0%> (-0.35%) ⬇️
p2p/peer.go 57.95% <0%> (ø) ⬆️
privval/socket.go 72.28% <0%> (+0.33%) ⬆️
libs/db/remotedb/remotedb.go 41.52% <0%> (+4.68%) ⬆️

@ebuchman ebuchman merged commit 728d2ed into develop Aug 14, 2018
@ebuchman ebuchman deleted the dev/fix_crypto_prefixing branch August 14, 2018 23:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants