-
Notifications
You must be signed in to change notification settings - Fork 22
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
feat(cmd): edit finality provider description #90
Conversation
Could you share the flag error it was giving? |
@RafilxTenfen it's inheriting the flags passed, we pass in the "daemon-address" flag needed for FP CLI, which also gets passed to the Babylon command, resulting in the unexpected flag. But anyways, I find this approach better, as you are handling things on the "server" rather than on the "client". As of now CLI just calls the RPC where everything is handled. In the other approach where the CLI would:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In my mind it would be easier to take advantage of the CLI that already exists on Babylon like #92 and add a post run to update the local storage of fpd if needed
But, since this is already developed and it does something more than just call the transaction (set default values) the code looks good to me
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lgtm! Some minor comments:
Seems CI was not triggered? |
prob because of conflict with the changelog |
Note: I did try using the edit cmd defined in Babylon, but I wasn't successful as the cmd context is getting passed into the edit cmd resulting in a flags error.
References issue