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

Added map of string to codecs #26

Merged
merged 3 commits into from Jun 30, 2017
Merged

Added map of string to codecs #26

merged 3 commits into from Jun 30, 2017

Conversation

ghost
Copy link

@ghost ghost commented Jun 30, 2017

Example

cid.Codecs["eth-block"] // Will evaluate to '\x90' (cid.EthBlock)

Very useful for ipfs block put --format <format> --mhtype <mhtype> <object>

@ghost ghost mentioned this pull request Jun 30, 2017
@coveralls
Copy link

Coverage Status

Coverage increased (+63.8%) to 63.846% when pulling f699041 on hermanjunge:feat/codecs-map into 3f7f6c6 on ipfs:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage increased (+63.8%) to 63.846% when pulling f699041 on hermanjunge:feat/codecs-map into 3f7f6c6 on ipfs:master.

Copy link
Member

@Stebalien Stebalien left a comment

Choose a reason for hiding this comment

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

LGTM

cid_test.go Outdated
for k, v := range tCodes {
if Codecs[v] != k {
t.Errorf("Table mismatch: 0x%x %s", k, v)
}
Copy link
Member

Choose a reason for hiding this comment

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

There should probably be a len(tCodes) == len(Codecs) - 1 test here.

Copy link
Author

Choose a reason for hiding this comment

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

@Stebalien improved the tests. Added the one requested.

@coveralls
Copy link

coveralls commented Jun 30, 2017

Coverage Status

Coverage increased (+63.8%) to 63.846% when pulling 3c03b9e on hermanjunge:feat/codecs-map into 3f7f6c6 on ipfs:master.

cid.go Outdated
@@ -79,6 +79,28 @@ const (
ZcashTx = 0xc1
)

// Codecs maps the name of a codec to its type
var Codecs = map[string]byte{
Copy link
Member

Choose a reason for hiding this comment

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

I am not sure that byte is right format for it.
The codecs are not limited to a byte (due to varint) we just are trying to keep codecs that are being used in cid in the 8 bit range but it might not be much longer till we will have two byte CID codecs (we already have two and three byte multihashes).

Copy link
Member

Choose a reason for hiding this comment

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

It should probably be uint64 as in https://github.com/ipfs/go-cid/blob/master/cid.go#L109

Copy link
Author

Choose a reason for hiding this comment

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

@Kubuxu fixed

@coveralls
Copy link

Coverage Status

Coverage increased (+63.8%) to 63.846% when pulling 7d345d4 on hermanjunge:feat/codecs-map into 3f7f6c6 on ipfs:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage increased (+63.8%) to 63.846% when pulling 7d345d4 on hermanjunge:feat/codecs-map into 3f7f6c6 on ipfs:master.

@whyrusleeping whyrusleeping merged commit e449699 into ipfs:master Jun 30, 2017
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.

None yet

4 participants