-
-
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
block cmd: Use coreapi #5331
block cmd: Use coreapi #5331
Conversation
97ffa39
to
988fb7a
Compare
Blocked by a bug in go-path |
cdf58f1
to
e22dd35
Compare
b7b84ce
to
301fcf8
Compare
}) | ||
rp, err := api.ResolvePath(req.Context, p) | ||
if err != nil { | ||
res.SetError(err, cmdkit.ErrNormal) |
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.
Should we emit error here too?
What do you think?
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.
Reading original code we probably shouldn't unless ResolvePath
does a lot more than just Decode
.
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.
It does handle ipns and path traversal too
core/coreapi/block.go
Outdated
settings.Codec = "v0" | ||
} | ||
} | ||
|
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.
Should this be in BlockPutOptions
instead, it seems like it is options handling.
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 way we separate Options sanitation from implementation.
a2955d0
to
dc5d66c
Compare
dc5d66c
to
cccd98c
Compare
License: MIT Signed-off-by: Łukasz Magiera <magik6k@gmail.com>
License: MIT Signed-off-by: Łukasz Magiera <magik6k@gmail.com>
License: MIT Signed-off-by: Łukasz Magiera <magik6k@gmail.com>
License: MIT Signed-off-by: Łukasz Magiera <magik6k@gmail.com>
cccd98c
to
1f98f4b
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 for now. However, the interface will likely change when we start addressing blocks by hash instead of by CID.
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, let's get it in.
No description provided.