-
-
Notifications
You must be signed in to change notification settings - Fork 3k
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
commands/filestore: use new cmds lib #5673
Conversation
672dfd0
to
28fc08a
Compare
There seams to be a lot of changes here. Please give me a few days to look this over. |
794e8c7
to
d67709a
Compare
core/commands/filestore.go
Outdated
@@ -150,7 +151,7 @@ For ERROR entries the error will also be printed to stderr. | |||
Encoders: cmds.EncoderMap{ | |||
cmds.Text: cmds.MakeTypedEncoder(func(req *cmds.Request, w io.Writer, out *filestore.ListRes) error { | |||
if out.Status == filestore.StatusOtherError { | |||
return fmt.Errorf(out.ErrorMsg) | |||
fmt.Fprintf(os.Stderr, "%s\n", out.ErrorMsg) |
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.
This might be okay. Although i think we need to use PostRun here. I created an issue for it n go-ipfs-cmds
: ipfs/go-ipfs-cmds#133.
Blocking for now to until others weigh in.
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.
@Stebalien confirmed in ipfs/go-ipfs-cmds#133 that we should be using PostRun here.
d67709a
to
ed40684
Compare
@overbool do you want me to take over this commit and fix it to use PostRun? |
ed40684
to
25bba79
Compare
Ok, thx. |
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.
One minor thing then LGTM.
core/commands/filestore.go
Outdated
if !ok { | ||
return nil, e.TypeErr(r, v) | ||
} | ||
list := v.(*filestore.ListRes) |
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.
This should be:
list, ok := v.(*filestore.ListRes)
if !ok {
return nil, e.TypeErr(r, v)
}
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.
@kevina I had changed it, thx
Sorry. I didn't see that you already converted the command to use PostRun. One minor change then it LGTM. |
25bba79
to
55572fe
Compare
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
License: MIT Signed-off-by: Overbool <overbool.xu@gmail.com>
License: MIT Signed-off-by: Overbool <overbool.xu@gmail.com>
License: MIT Signed-off-by: Overbool <overbool.xu@gmail.com>
License: MIT Signed-off-by: Overbool <overbool.xu@gmail.com>
License: MIT Signed-off-by: Overbool <overbool.xu@gmail.com>
55572fe
to
b4b6fc6
Compare
Refer: #5664
License: MIT
Signed-off-by: Overbool overbool.xu@gmail.com