refactor: update apps to use gRPC #20
Conversation
jeronimoalbi
requested review from
ilgooz,
Pantani,
julienrbrt and
Ehsan-saradar
as code owners
| // some other sub commands related to hermes relayer. | ||
| func NewRelayer() *cobra.Command { | ||
| func NewHermes() *cobra.Command { |
There was a problem hiding this comment.
Maybe create an alias for not breaking the API?
There was a problem hiding this comment.
I changed the name to NewHermes because the command is called "hermes".
It is attached to the ignite relayer "namespace" to allow other relayers, so NewHermes is called when ignite relayer hermes is executed. Maybe in the future we might have something like ignite relayer go for example. So "relayer" is a namespace for IBC relayers while "hermes" is the name of the actual command.
I'm not sure what you mean by breaking the API
There was a problem hiding this comment.
Why the go.work.sum is committed?
There was a problem hiding this comment.
It was already present in the repo and I think it was updated in this PR because some Go mod dependencies were updated.
I'm not sure about this but maybe workspaces should not be committed in a repo, or at least not the workspace sum file. Maybe you mean that just the go.work.sum should be omitted?
There was a problem hiding this comment.
We had a long discussion in the SDK about not committing go.work and go.work.sum at all in the repo.
go.work is in the default GitHub gitignore for instance: https://github.com/github/gitignore/blob/main/Go.gitignore and the go.work.sum (github/gitignore#4081) will be one day.
There was a problem hiding this comment.
+1 for not committing workspaces at all
Requires ignite/cli#3529 to be merged first.