Skip to content
This repository has been archived by the owner on Aug 11, 2021. It is now read-only.

Binary resolver #90

Merged
merged 11 commits into from
Oct 7, 2017
Merged

Binary resolver #90

merged 11 commits into from
Oct 7, 2017

Conversation

kumavis
Copy link
Contributor

@kumavis kumavis commented Jul 13, 2017

currently set to base2 but per discussion this might change

@daviddias daviddias added the status/in-progress In progress label Jul 13, 2017
Copy link
Member

@daviddias daviddias left a comment

Choose a reason for hiding this comment

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

Let's move this forward :)

src/index.js Outdated
@@ -23,6 +23,7 @@ const ipldEthStateTrie = require('ipld-eth-star').ethStateTrie
const ipldEthStorageTrie = require('ipld-eth-star').ethStorageTrie
const ipldEthTx = require('ipld-eth-star').ethTx
const ipldEthTxTrie = require('ipld-eth-star').ethTxTrie
const ipldBin = require('./ipldBin')
Copy link
Member

Choose a reason for hiding this comment

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

@kumavis mind moving this resolver to here https://github.com/ipld/js-ipld-raw ?

src/ipldBin.js Outdated
// binary resolver
module.exports = {
resolver: {
multicodec: 'base2',
Copy link
Member

Choose a reason for hiding this comment

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

I remember that discussed this in great length and the reasons why raw is different from base2. If I remember correctly, you pointed out that base2 is actually incorrect .

I'm not finding the issue where that discussion happened (I think it was in IRC, our bad for not logging it).

All of that said, let's use the codec raw here just to avoid any future confusion https://github.com/multiformats/multicodec/blob/master/table.csv#L3

Copy link

Choose a reason for hiding this comment

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

Base2 codec is literal stream of ASCI: 010100111010110.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Kubuxu does base2 have a code yet? in the multiformat table it has the same value as bin

@daviddias
Copy link
Member

ping @kumavis

@kumavis
Copy link
Contributor Author

kumavis commented Sep 5, 2017

wubbalubbadubdub ok I'll get back on this horse soon
i need more me's

@ghost
Copy link

ghost commented Oct 4, 2017

@kumavis you need mr meeseeks

@daviddias
Copy link
Member

@kumavis did something change in ipld-ethereum account?

See: https://travis-ci.org/ipld/js-ipld-resolver/jobs/284172920#L2356-L2373

Copy link
Member

@daviddias daviddias left a comment

Choose a reason for hiding this comment

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

needs to check failing tests

@kumavis
Copy link
Contributor Author

kumavis commented Oct 6, 2017

Not sure about the CI test failures. Some CI passed, tests pass locally for me.

@daviddias daviddias merged commit 2968681 into master Oct 7, 2017
@daviddias daviddias deleted the bin-resolver branch October 7, 2017 16:45
@kumavis
Copy link
Contributor Author

kumavis commented Oct 9, 2017

Thanks~

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants