-
Notifications
You must be signed in to change notification settings - Fork 124
Conversation
daviddias
commented
Nov 23, 2016
607b750
to
85acd78
Compare
|
||
series([ | ||
(cb) => { | ||
node1.addNodeLink('some-link', node2, cb) | ||
DAGNode(new Buffer('Some data 1'), (err, node) => { |
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.
Should this be DAGNode.create()?
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.
absolutely correct, missed 2
}, | ||
(cb) => { | ||
ipfs.object.put(node1, (err, node) => { | ||
DAGNode(new Buffer('Some data 2'), (err, node) => { |
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.
Should this be DAGNode.create()?
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.
thank you, just fixed now with more testing :)
Couple of questions. Otherwise looks good to me 👍 |
node2 = node | ||
cb() | ||
}) | ||
}, |
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.
Same as in ipld/js-ipld#67 (comment)
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.
True, it can save us some lines, however, I don't dislike how it is right now, it makes it very easy to read (I believe). Just for the sake of time (and wanting to have this before the hackathon), will leave this reconsideration for after.
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.
I find it super hard to read :/ as it is spread on so many lines that I have difficulty figuring out what is actually happening
node2 = node | ||
cb() | ||
}) | ||
}, |
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.
Same as in ipld/js-ipld#67 (comment)
expect(err).to.not.exist | ||
node2 = node | ||
cb() | ||
}) |
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.
Same as in Same as in ipld/js-ipld#67 (comment)
85acd78
to
519c944
Compare
Thank you for the review :) |