feat(textual): Coin and Coins value renderers
#12729
Conversation
| [{"amount": "100000", "denom": "point"}, { "denom": "POINT", "exponent": 0 }, "100'000 POINT"], | ||
| [{"amount": "1000000", "denom": "point"}, { "denom": "POINT", "exponent": 0 }, "1'000'000 POINT"], | ||
| [{"amount": "10000000", "denom": "point"}, { "denom": "POINT", "exponent": 0 }, "10'000'000 POINT"] | ||
| ] |
There was a problem hiding this comment.
Add a test cases for
- zero amount
- coin without metadata
- metadata with entries other than display and coin names
- metadata without the display or coin name
- failures (see
timestamp.jsonfor examples)
There was a problem hiding this comment.
Added test cases for all items except "metadata with entries other than display and coin names". That would be a test-only-related json parsing error, which I don't think is really useful here?
There was a problem hiding this comment.
Additional metadata entries would be valid JSON data, and shouldn't be an SDK error either. As I understand metadata, it should be perfectly valid to have:
"metadata": {
"display": "buck",
"denom_units": [
{"denom": "ubuck", "exponent": 0},
{"denom": "mill", "exponent": 3},
{"denom": "cent", "exponent": 4},
{"denom": "dime", "exponent": 5},
{"denom": "buck", "exponent": 6},
{"denom": "grand", "exponent": 9},
{"denom": "dirksen", "exponent": 15}
]
}
There was a problem hiding this comment.
OK, I misunderstood, by metadata with entries other than display I understood adding e.g. a field foo: ... in the metadata struct.
Added test cases with unused items in the denom_units array.
| "uxc1": { "denom": "xC1", "exponent": 6 } | ||
| }, | ||
| "0.000001 xA, 0.000002 xB, 0.000003 xC, 0.000004 xC1" | ||
| ] |
There was a problem hiding this comment.
Add test cases for an empty Coins and a Coins of a single Coin.
Also add cases with a failure in coins at the beginning, middle, and end or the coins array.
There was a problem hiding this comment.
Still needs empty coins and singleton coins.
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #12729 +/- ##
==========================================
- Coverage 55.87% 55.09% -0.79%
==========================================
Files 646 654 +8
Lines 54895 55627 +732
==========================================
- Hits 30675 30648 -27
- Misses 21762 22518 +756
- Partials 2458 2461 +3
|
Co-authored-by: Facundo Medica <14063057+facundomedica@users.noreply.github.com>
…os-sdk into am/12708-textual-coins
amaury1093
added
the
A:automerge
|
I'm putting automerge on, to unblock some auto-generated CLI work which needs the coin formatting function. @JimLarson I believe I addressed all your comments, but can do a follow-up if there's something else left. |
Description
Closes: #12708
Initially was part of #12088, but carved out of that PR into its own.
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...
!to the type prefix if API or client breaking changeCHANGELOG.mdReviewers 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...
!in the type prefix if API or client breaking change