commands/boostrap: use new cmds #5678
Conversation
There was a problem hiding this comment.
Seems to make tests fail: https://ci.ipfs.team/blue/organizations/jenkins/IPFS%2Fgo-ipfs/detail/PR-5678/2/tests
core/commands/bootstrap.go
Outdated
| } | ||
|
|
||
| r, err := fsrepo.Open(req.InvocContext().ConfigRoot) | ||
| ctx := env.(*oldcmds.Context) |
There was a problem hiding this comment.
Not a requirement, but we should likely create another helper function in core/commands/cmdenv and call it GetConfigRoot. This way it makes it clear that the cast is the right thing to do. It would of saved me 10-15 minutes of review time.
There was a problem hiding this comment.
Due to an Oddity with how parsing arguments from Stdin when ever you have a command that can accept more than one argument from Stdin you must either call req.ParseBodyArgs() before using req.Arguments or iterate over them with req.BodyArgs(), with the former being preferred for now. If you don't only the first argument will be read from Stdin.
You can test this my trying to pass two arguments via the command line to an affected command and then try again via Stdin.
License: MIT Signed-off-by: Overbool <overbool.xu@gmail.com>
overbool
force-pushed
the
refactor/commands/bootstrap
branch
from
25cb875
to
6ee5b09
Compare
License: MIT Signed-off-by: Overbool <overbool.xu@gmail.com>
overbool
force-pushed
the
refactor/commands/bootstrap
branch
from
6ee5b09
to
bc4a9ef
Compare
overbool
changed the title
|
@kevina I had fixed them, could u help me review it again? |
Refer: #5664
License: MIT
Signed-off-by: Overbool overbool.xu@gmail.com