-
-
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
fix(cmds): CIDv1 and correct multicodecs in 'block put' and 'cid codecs’ #8568
Merged
+204
−58
Merged
Changes from 8 commits
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
4448de5
fix(cmds): use correct ipld multicodecs in cmds
schomatis e348b59
refactor: block put --cid-codec
lidel 70c13cf
chore: go-multicodec v0.4.1 + helptext fixes
lidel a33ede2
fix: add libp2p-key to supported codecs
lidel 2119931
fix(test): t0110-gateway.sh
lidel 44cc485
test: interface-go-ipfs-core CIDv1 raw and dag-pb
lidel 02aff74
fix(test): t0290-cid.sh
lidel c34fe9c
test: use of both 'format' and 'cid-codec'
lidel 2f5b8a4
docs(changelog): document breaking changes
lidel 1f4902b
chore: interface-go-ipfs-core v0.7.0
lidel File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
@aschmahmann I did some tests around this and the good news is that I was wrong, there is a way to have unified handling of default opts that works the same in
ipfs block put
CLI and raw HTTP POST to/api/v0/block/put
.If we keep the
.WithDefault("raw")
here, then this implicit default is also applied when people use curl against/api/v0/block/put
and forget to passcid-codec
. It will also produce good docs at https://docs.ipfs.io/reference/http/api/#api-v0-block-put.If we want to be extra strict and return error, I'd have to remove
WithDefault
and add some glue code that detectscidCodec == ""
and throws error, but that feels hacky and annoying because it would makecid-codec
a mandatory parameter, making it bad UX for the common use case where people are fine with codec being raw.My vote is to keep
WithDefault("raw")
.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.
Yep, the only breaking change is:
Which only effects the output CID, but
ipfs block put <some-dagpb-stuff>; ipfs block get QmDagPBv0Thing
will still work fine. This seems like a not too bad silent breakage, although I'm generally averse to silent breakages. WDYT?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.
sgtm. Added 0.13 breaking changes section to CHANGELOG.md in 2f5b8a4