Make cli more robust #36
Conversation
commands/init.go
Outdated
| // make sure there is a directory here in any case | ||
| os.MkdirAll(root, dirPerm) | ||
|
|
||
| // check there is a config.toml file |
There was a problem hiding this comment.
// check if there is a config.toml file
commands/init.go
Outdated
| defer d.Close() | ||
|
|
||
| // read to see if any files here... | ||
| files, err := d.Readdirnames(5) |
There was a problem hiding this comment.
Could you add a comment about the argument you are passing in.
commands/init.go
Outdated
| return false, errors.WithStack(err) | ||
| } | ||
|
|
||
| // check there are non-empty checkpoints and validators dirs |
commands/init.go
Outdated
| // return err | ||
| // } | ||
| // | ||
| // I tried using PersistentPreRun, |
There was a problem hiding this comment.
Let's try to reduce the use of first-person. "This can't just be called during PersistentPreRun because ..."
Also, do we really need an example of calling the function and returning an error ?
| return nil | ||
| } | ||
|
|
||
| doc, err := tcmd.ParseGenesisFile(genesis) |
There was a problem hiding this comment.
Should this really be in tendermint/cmd/commands ? Would it serve better in tendermint/types ?
There was a problem hiding this comment.
I can move, tried to do minimal changes in tendermint (i'm scared to touch too much).
If you comment on that issue, I will update this: tendermint/tendermint#561
commands/proofs/tx.go
Outdated
| @@ -22,6 +22,10 @@ data to other peers as needed. | |||
| } | |||
|
|
|||
| func doTxQuery(cmd *cobra.Command, args []string) error { | |||
| if err := commands.RequireInit(cmd); err != nil { | |||
There was a problem hiding this comment.
not thrilled that we have to do this with every cmd func. Can we not move it into root command, so it only needs to be called in one place ? Or otherwise have some wrapper convention ? I'm worried new commands will neglect it ...
There was a problem hiding this comment.
There is a problem, as some commands (eg. init and keys) do not require Init.
I tried to add it to the query root command via PreRunE, but that didn't work, as you noticed in the comment above. If you have a better idea, I am totally game. It hurts me a bit to copy this all over....
Only other idea was to have a decorator, that takes RunE and returns a RunE with this prefix. But I thought this was too much magic from the python world. but happy to do.
There was a problem hiding this comment.
RunE: RequireInit(runQuery)
There was a problem hiding this comment.
That was my other thought. I'd rather that RequireInit(runQuery) piece than copying this everywhere :)
There was a problem hiding this comment.
Okay, will update both repos.
test/init.sh
Outdated
| } | ||
|
|
||
| oneTimeTearDown() { | ||
| printf "\nstoping tendermint..." |
test/init.sh
Outdated
| assertFalse "ls ${TMHOME} 2>/dev/null >&2" | ||
|
|
||
| # no node where we go | ||
| echo y | tmcli init --node=tcp://localhost:9999 --chain-id="bad-chain-id" > /dev/null 2>&1 |
There was a problem hiding this comment.
Not that it matters here but we should only change one thing at a time. ie. we should use the correct chain-id if we're using the wrong node
| assertFalse "warning on re-init" $? | ||
| checkDir $TMHOME 3 | ||
|
|
||
| # unless we --force-reset |
test/init.sh
Outdated
| @@ -27,7 +27,7 @@ oneTimeSetUp() { | |||
| } | |||
|
|
|||
| oneTimeTearDown() { | |||
| printf "\nstoping tendermint..." | |||
| printf "\nstoping..." | |||
There was a problem hiding this comment.
I was trying to indicate that "stopping" is spelled wrong, not that we shouldn't say "tendermint" :)
This addresses these issues
#35
#19