ica docs improvements #2892
ica docs improvements #2892
Conversation
crodriguezvega
requested review from
colin-axner,
AdityaSripal,
damiannolan,
charleenfei and
chatton
as code owners
| Example: | ||
|
|
||
| ```shell | ||
| simd query interchain-accounts controller interchain-account cosmos1.. connection-0 |
There was a problem hiding this comment.
nit: should we use a full bech32 address in examples?
There was a problem hiding this comment.
I don't have a strong preference. I used this because I saw it in the SDK docs and I thought it removed a bit of noise, but happy to add the full address if people think it's better.
| @@ -23,24 +23,16 @@ This message is expected to fail if: | |||
|
|
|||
| This message will construct a new `MsgChannelOpenInit` on chain and route it to the core IBC message server to initiate the opening step of the channel handshake. | |||
|
|
|||
| The controller module will generate a new port identifier and claim the associated port capability. The caller is expected to provide an appropriate application version string. For example, this may be an ICS27 JSON encoded [`Metadata`](https://github.com/cosmos/ibc-go/blob/v6.0.0-alpha1/proto/ibc/applications/interchain_accounts/v1/metadata.proto#L11) type or an ICS29 JSON encoded [`Metadata`](https://github.com/cosmos/ibc-go/blob/v6.0.0-alpha1/proto/ibc/applications/fee/v1/metadata.proto#L11) type with a nested application version. | |||
| If the `Version` string is omitted, the application will construct a default version string in the `OnChanOpenInit` handshake callback. | |||
| The controller submodule will generate a new port identifier and claim the associated port capability. The caller is expected to provide an appropriate application version string. For example, this may be an ICS-27 JSON encoded [`Metadata`](https://github.com/cosmos/ibc-go/blob/v6.0.0-rc0/proto/ibc/applications/interchain_accounts/v1/metadata.proto#L11) type or an ICS29 JSON encoded [`Metadata`](https://github.com/cosmos/ibc-go/blob/v6.0.0-rc0/proto/ibc/applications/fee/v1/metadata.proto#L11) type with a nested application version. | |||
There was a problem hiding this comment.
Are we now using dash separate ICS numbers? It's really unclear to me on what should be used.
| The controller submodule will generate a new port identifier and claim the associated port capability. The caller is expected to provide an appropriate application version string. For example, this may be an ICS-27 JSON encoded [`Metadata`](https://github.com/cosmos/ibc-go/blob/v6.0.0-rc0/proto/ibc/applications/interchain_accounts/v1/metadata.proto#L11) type or an ICS29 JSON encoded [`Metadata`](https://github.com/cosmos/ibc-go/blob/v6.0.0-rc0/proto/ibc/applications/fee/v1/metadata.proto#L11) type with a nested application version. | |
| The controller submodule will generate a new port identifier and claim the associated port capability. The caller is expected to provide an appropriate application version string. For example, this may be an ICS-27 JSON encoded [`Metadata`](https://github.com/cosmos/ibc-go/blob/v6.0.0-rc0/proto/ibc/applications/interchain_accounts/v1/metadata.proto#L11) type or an ICS-29 JSON encoded [`Metadata`](https://github.com/cosmos/ibc-go/blob/v6.0.0-rc0/proto/ibc/applications/fee/v1/metadata.proto#L11) type with a nested application version. |
There was a problem hiding this comment.
I understand it's confusing and apologies for that. My proposal would be to either use - or a space. I will add a line about this in the docs guidelines PR. At some point I will open a PR to update the whole repository according to that standard.
| @@ -23,7 +23,7 @@ import ( | |||
|
|
|||
| // Please use MsgRegisterInterchainAccount for use cases which do not need to route to an underlying application. | |||
|
|
|||
| // Prior to to v6.x.x of ibc-go, the controller module was only functional as middleware, with authentication performed | |||
There was a problem hiding this comment.
this fix caused all the e2e tests to run :D
|
|
||
| A user can query and interact with the Interchain Accounts module using the CLI. | ||
|
|
||
| ### Controller |
There was a problem hiding this comment.
I personally think this section is mostly unnecessary. CLI should be self documenting. I don't think it makes a lot of sense to add extra documentation, unless that documentation is explaining how to accomplish X using a series of commands
There was a problem hiding this comment.
I see your point, and I agree the CLI is pretty self documenting. But I have noticed from some users on Discord that sometimes it seems like they are not aware of the CLI commands we provide, so that's why I thought having something like this might help...
There was a problem hiding this comment.
Could we slim the docs down so if we make changes to the cli we don't have to update these docs as well (ie maintain docs in two places)?
I think it is fine to bring visibility to the CLI and explain how to obtain more information on how to use the CLI, but I'm sure an equal amount of individuals wouldn't realize there is more docs on the CLI outside of the self documentation obtainable via the --help command. I'd prefer to consolidate effort by making those commands have sufficient information
There was a problem hiding this comment.
All right, I trimmed down this file. I left explanation only for the send-tx and generate-packet-data commands because I think there's an interplay between those 2 that is not immediately obvious from the command line. Please have a look!
…to carlos/ica-docs
Description
closes: #XXXX
Commit Message / Changelog Entry
see the guidelines for commit messages. (view raw markdown for examples)
Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.
docs/) or specification (x/<module>/spec/).godoccomments.Files changedin the Github PR explorer.Codecov Reportin the comment section below once CI passes.