ipfs / go-cid

Content ID v1 implemented in go
MIT License
157 stars 47 forks source link

Add the dagjose multiformat #112

Closed alexjg closed 4 years ago

welcome[bot] commented 4 years ago

Thank you for submitting this PR! A maintainer will be here shortly to review it. We are super grateful, but we are also overloaded! Help us by making sure that:

Getting other community members to do a review would be great help too on complex PRs (you can ask in the chats/forums). If you are unsure about something, just leave us a comment. Next steps:

We currently aim to provide initial feedback/triaging within two business days. Please keep an eye on any labelling actions, as these will indicate priorities and status of your contribution. We are very grateful for your contribution!

alexjg commented 4 years ago

I guess that's so that client code can generically go from codec name to the actual multicodec value? E.g this code in go-ipfs:

        if codecStr != "" {
            codec, ok := cid.Codecs[codecStr]
            if !ok {
                return fmt.Errorf("unknown IPLD codec: %s", codecStr)
            }
            opts.newCodec = codec
        } // otherwise, leave it as 0 (not a valid IPLD codec)
alexjg commented 4 years ago

Hey folks, is there anything more that needs doing on this? Would be great to get it merged.

rvagg commented 4 years ago

I've just realised I have merge access on this repo so could get this merged. There's something wrong with the codecov report though and I can't re-run it. Would you mind bumping the commit so it'll re-run please? The repo rules stop me overriding it unfortunately.

alexjg commented 4 years ago

Hmm, I've tried bumping the commit and it looks like it's still causing problems. I can't really figure out why there are coverage changes, the codecov output doesn't seem particularly enlightening.

rvagg commented 4 years ago

Your commit accidentally slipped in via #115 (I must have been messing around with trying to get this in from the cmdline and had it on my master so it slipped in via a rebase). But that's OK because I was trying to get it in anyway! Done now.