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.
The tag should be
serialization
as by default it doesn't support links (CIDs). After a quick look, I've seen that BARE supports user defined types. Would the plan be to specify a user defined type for 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.
Yes. If the user wants CIDs, it would be very straightforward to define one.
Because BARE is not self-describing, user-defined types have no overhead and are no different than builtins.
I see
json
markedipld
on line 133, but the official JSON RFC(s) don't specify support links. Is this possibly a mistake, confused withdag-json
?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 then perhaps the information about which one to use be defined somewhere?
json
(and others) should really beserialization
, it just grown historically. We even still haven't it properly written down what qualifies anipld
codec.. No one took the time to look over the whole table and make the call. Though I'd like to get it "as right as possible" for new additions.What emerged over the past few months (I also touch it a bit at #204 (comment)) is: The
codec
field in CIDs is about "how do I process this data in order to get links/CIDs out". And currently I see theipld
tag as codes that qualify for that.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.
@Techcable the current position is something like this - if a serialization format doesn't potentially emit a
Link
(CID) type then we're calling it aserialization
, but if it does emitLink
s and therefore allow us to form a graph of connected blocks then we're calling itipld
. The existing entries that areipld
that don't match this are historical and we're probably going to change that. So for now this would be appropriate to beserialization
. If you could change that we can probably merge it.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.
@Techcable : are you able to incorporate feedback here so we can get this merged? Thanks!