Skip to content
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

Support encoing using IPLD-Prime codecs #209

Closed
wants to merge 2 commits into from
Closed

Conversation

willscott
Copy link
Contributor

Allows for an argument of --encoding=dag-json or other valid ipld-prime codecs to be used in commands like dag get, where a dag node is being encoded to stdout.

Note: this relies on ipfs/kubo#7976 so that the node returned from the command is an ipld prime node that the encoder can successfully encode. When the return value is not an ipld prime node, and an ipld-prime codec is chosen, an error that an invalid node time has been attempted to be encoded is emitted.

@BigLep BigLep requested a review from warpfork May 17, 2021 15:45
Copy link
Member

@warpfork warpfork left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As a novice reviewer to this repo -- this looks fine to me.

Comment on lines +199 to +205
// In addition to the explicitly named legacy encoders defined in this file,
// support encoding with the encoders registered in the ipld-prime multicodec
// registry.
ipldCodec, ok := mc.Of(string(encType))
if !ok {
if !ok {
n, err := strconv.Atoi(string(encType))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like two new features:

  • more string names supported
  • base-10 numbers supported

Are there docs changes that should go with it?

@willscott
Copy link
Contributor Author

we no longer need this PR based on the current state of ipfs/kubo#7995

@willscott willscott closed this Jul 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants