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

Reuse code for parsing integer slices from string #722

Closed
ValarDragon opened this issue Jan 6, 2022 · 4 comments · Fixed by #747
Closed

Reuse code for parsing integer slices from string #722

ValarDragon opened this issue Jan 6, 2022 · 4 comments · Fixed by #747

Comments

@ValarDragon
Copy link
Member

There are several places in the codebase (CLI folders) where we ad-hoc write parsing of a comma-delimited string into []uint64 ro []sdk.Int. We should have a single method we call for this, not ad-hoc implemented in line.

The function for []uint64 is written here:

// TODO: move somewhere common, make more commands use it
func parseUint64SliceFromString(s string, seperator string) ([]uint64, error) {
var ids []uint64
for _, s := range strings.Split(s, seperator) {
s = strings.TrimSpace(s)
base := 10
bitlen := 64
parsed, err := strconv.ParseUint(s, base, bitlen)
if err != nil {
return []uint64{}, err
}
ids = append(ids, parsed)
}
return ids, nil
}

We should move that function and write one for SDK.Int in
https://github.com/osmosis-labs/osmosis/blob/main/osmotestutils/cli_helpers.go

We should then switch balances_from_state_export, and pool incentives CLI's to use this.

This is implemented in-line in the pool-incentives CLI code here: https://github.com/osmosis-labs/osmosis/blob/main/x/pool-incentives/client/cli/tx.go#L49-L69 & https://github.com/osmosis-labs/osmosis/blob/main/x/pool-incentives/client/cli/tx.go#L143-L163

@p0mvn
Copy link
Member

p0mvn commented Jan 10, 2022

Looking into this...

@iButcat
Copy link
Contributor

iButcat commented Jan 10, 2022

Oups sorry, I forgot to say I was on this as well. @akhtariev

@p0mvn
Copy link
Member

p0mvn commented Jan 10, 2022

Ok, no worries @iButcat

@iButcat
Copy link
Contributor

iButcat commented Jan 10, 2022

@ValarDragon, Hi, I think that's good, can you review my PR please ?

Repository owner moved this from 🕒 Todo to ✅ Done in Osmosis Chain Development Jan 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants