chore: move FormatCoins to x/tx #18857
Conversation
WalkthroughThe codebase underwent a refactoring where the functionality for formatting and sorting coin values was moved from the Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on X ? TipsChat with CodeRabbit Bot (
|
|
|
||
| return strings.Join(formatted, ", "), nil | ||
| } | ||
|
|
||
| // ParseCoin parses a coin from a string. The string must be in the format | ||
| // <amount><denom>, where <amount> is a number and <denom> is a valid denom. | ||
| func ParseCoin(input string) (*basev1beta1.Coin, error) { |
There was a problem hiding this comment.
ParseCoin is being left here as it's being used in autocli. textual has its own parsing, which is only used in tests
There was a problem hiding this comment.
@julienrbrt what do you think of moving this to client/v2? do we see this being used anywhere else in the future?
There was a problem hiding this comment.
Sure, we can move that into client/v2 (please in the internal package however)
| // alphabetically by value-rendered denoms. It expects an array of metadata | ||
| // (optionally nil), where each metadata at index `i` MUST match the coin denom | ||
| // at the same index. | ||
| func FormatCoins(coins []*basev1beta1.Coin, metadata []*bankv1beta1.Metadata) (string, error) { |
There was a problem hiding this comment.
Can we get a changelog in core, as this is breaking?
There was a problem hiding this comment.
pushed, thank you!!
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
|
|
||
| ### Improvements | ||
|
|
||
| * [#18857](https://github.com/cosmos/cosmos-sdk/pull/18857) Moved `FormatCoins` from `core/coins` to this package under `signing/textual`. |
There was a problem hiding this comment.
The entry for moving FormatCoins is currently listed under "Improvements". Given that this is a relocation of a public function to a different package, it should be considered an "API Breaking" change. Please ensure that this entry is placed under the correct stanza to accurately reflect the nature of the change.
There was a problem hiding this comment.
It's breaking for the core package but an improvement for the x/tx package imo
I was going to push it here too but I was too slow lol, I'll open a new PR in a bit |
|
pushed #18861 |
Description
Moving
FormatCoinsto x/tx/signing/textual given that that's the only place where it's being used.Closes: #XXXX
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...
!in 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...