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

bank: name and symbol metadata fields #8677

Merged
merged 7 commits into from
Feb 24, 2021
Merged

Conversation

fedekunze
Copy link
Collaborator

@fedekunze fedekunze commented Feb 23, 2021

closes #8614

cc: @jtremback

@fedekunze fedekunze added the A:automerge Automatically merge PR once all prerequisites pass. label Feb 23, 2021
@codecov
Copy link

codecov bot commented Feb 23, 2021

Codecov Report

Merging #8677 (3f5c742) into master (77668a3) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #8677   +/-   ##
=======================================
  Coverage   61.48%   61.49%           
=======================================
  Files         659      659           
  Lines       38045    38049    +4     
=======================================
+ Hits        23393    23399    +6     
+ Misses      12193    12192    -1     
+ Partials     2459     2458    -1     
Impacted Files Coverage Δ
x/bank/types/metadata.go 100.00% <100.00%> (+5.55%) ⬆️

string name = 5;
// symbol is the token symbol usually shown on exchanges (eg: ATOM). This can
// be the same as the display.
string symbol = 6;
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we really need symbol? Can you give an example where symbol is not display upper/lower-cased?

Copy link
Collaborator Author

@fedekunze fedekunze Feb 24, 2021

Choose a reason for hiding this comment

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

@jtremback thoughts here? I'm fine with either tbh

Copy link
Contributor

Choose a reason for hiding this comment

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

BTC vs Bitcoin, ETH vs Ether, etc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A:automerge Automatically merge PR once all prerequisites pass. C:x/bank
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add Name and Symbol to Denom Metadata
5 participants