-
Notifications
You must be signed in to change notification settings - Fork 37
Migration to new block-service and block interface #81
Conversation
src/index.js
Outdated
// level: require('memdown') | ||
// }) | ||
// blockService = new BlockService(repo) | ||
// } |
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.
@nicola will not like 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.
but yeah, understood. Do what you gotta do.
@diasdavid the only failing test is because of https://github.com/dignifiedquire/js-ipld-eth-state-trie/tree/datastore There is a new ipfs block created in the resolver and I'm unsure what the right CID to put in there is. Could you take a look please? |
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.
Overall LGTM
- rm yarn.lock
- the eth-x need to be updated as well.
} | ||
callback(null, new IPLDResolver(blockService)) | ||
}) | ||
} |
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.
so that @nicola remains happy 🙌🏽
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.
this turned out to be great, as I had forgotten to test in-memory in nodejs and found a bug in the repo
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.
👍
@@ -25,10 +26,8 @@ function noop () {} | |||
|
|||
class IPLDResolver { | |||
constructor (blockService) { | |||
// nicola will love this! |
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.
poor @nicola
test/example-repo/datastore/LOG.old
Outdated
07:50:02.059983 db@janitor F·4 G·0 | ||
07:50:02.060001 db@open done T·2.755926ms | ||
07:50:02.073183 db@close closing | ||
07:50:02.073285 db@close done T·97.522µs |
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.
add to gitignore
hmm for the |
Sorry that i haven't been available to maintain the ipfs-eth stuff in a few weeks, hope to get back on it soon. |
Hmm seems i may have clobbered it here: dignifiedquire/js-ipld-eth-state-trie@bff107d |
@kumavis can we get the remaining ipld-eth-x resolvers under ipld org? It makes it easier for all of us to help you maintain it. Currently, it is blocking us from merging datastore all the way up. |
Things missing for this PR:
|
sure - who should i add as an npm owner? |
both me and @dignifiedquire (my handle is daviddias on npm http://npmjs.org/~daviddias) |
@kumavis @diasdavid as mentioned in irc I'm still not sure how to fix the module, could you two please put your heads together and make it so? <3 |
heres my status update:
|
All right, thank you @kumavis! Now for the final trick, we just need to update all of these to the new block:
|
test: call that callback now
🎉 🎉 🎉 🎉 🎉 🎉 |
Ref ipfs/js-ipfs#787