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

Add dag-jose, -cose #166

Merged
merged 2 commits into from
Apr 20, 2020
Merged

Add dag-jose, -cose #166

merged 2 commits into from
Apr 20, 2020

Conversation

oed
Copy link
Contributor

@oed oed commented Mar 20, 2020

See discussion here: ipld/specs#251

See discussion here: ipld/specs#251
@vmx
Copy link
Member

vmx commented Mar 20, 2020

@oed Could you please use a code > 0x80. Codes less that that are encoded as 1-byte varints and as there is limited space, we'd like to keep them for widely used formats.

@oed
Copy link
Contributor Author

oed commented Mar 20, 2020

Sure I can change it @vmx. I'm wondering though what defines "widely used formats"? Right now the plan is for 3Box and Textile to use these codecs for our dag structures.

@vmx
Copy link
Member

vmx commented Mar 20, 2020

I'm wondering though what defines "widely used formats"?

That's indeed a good question. We need to get more specific about that. Currently we try to push as much as possible outside the 0x80 range (historically sadly there things in that range that now wouldn't).

@vmx vmx requested a review from Stebalien March 20, 2020 16:51
Copy link
Member

@vmx vmx left a comment

Choose a reason for hiding this comment

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

Sounds good to me. @Stebalien any objections?

@rvagg
Copy link
Member

rvagg commented Apr 9, 2020

Do we need to see implementations and/or specifications before accepting something new into the table? Seems like there's a risk in having people stake out territory for things that don't exist yet. Maybe leaving an open PR like this while something is formalised is an appropriate course of action - flagging that these are wanted but waiting for something concrete to prove they exist outside of this table.

@vmx
Copy link
Member

vmx commented Apr 9, 2020

Do we need to see implementations and/or specifications before accepting something new into the table?

We don't, but soon we'll have a new column to indicate the state of things: #165

I'll work on this soon.

@mikeal
Copy link
Contributor

mikeal commented Apr 9, 2020

I do worry a bit about adding this particular codec before we’ve written a spec, but I’m not going to block the PR.

Ideally, our libraries would be much more tolerant of prototyping stuff like this using an unregistered entry so that we can start building and writing specs before committing an entry to the table. But we aren’t there yet, so lets get this in.

It’s pretty uncontroversial that we need a dag-cose codec since it uses another tag we currently forbid in dag-cbor.

@oed
Copy link
Contributor Author

oed commented Apr 9, 2020

Fyi, the js implementation of dag-jose is planned to start next week :)

@mikeal
Copy link
Contributor

mikeal commented Apr 9, 2020

Fyi, the js implementation of dag-jose is planned to start next week :)

Awesome! Once you have a PoC it would be great to get a spec for it in the specs repo, similar to what we have for other dag- codecs.

@vmx
Copy link
Member

vmx commented Apr 14, 2020

@Stebalien any objections to merge this one? I think it's good to go ("MerkleDAG" is weird, but that's also what the other codecs are currently using, I'd change that in a separate PR).

@vmx vmx merged commit 4422c4d into multiformats:master Apr 20, 2020
@vmx
Copy link
Member

vmx commented Apr 20, 2020

Finally. Sorry that merging this one took so long.

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.

4 participants