Skip to content
This repository was archived by the owner on Mar 10, 2020. It is now read-only.

Conversation

@daviddias
Copy link
Contributor

WIP

src/object.js Outdated
series([
(cb) => {
node1.addNodeLink('some-link', node2, cb)
DAGNode(new Buffer('Some data 1'), (err, node) => {
Copy link
Contributor

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()?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

absolutely correct, missed 2

src/object.js Outdated
},
(cb) => {
ipfs.object.put(node1, (err, node) => {
DAGNode(new Buffer('Some data 2'), (err, node) => {
Copy link
Contributor

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()?

Copy link
Contributor Author

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 :)

@haadcode
Copy link
Contributor

Couple of questions. Otherwise looks good to me 👍

node2 = node
cb()
})
},
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

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.

Copy link
Contributor

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()
})
},
Copy link
Contributor

Choose a reason for hiding this comment

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

expect(err).to.not.exist
node2 = node
cb()
})
Copy link
Contributor

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)

@daviddias
Copy link
Contributor Author

Thank you for the review :)

@daviddias daviddias merged commit 9871237 into master Nov 24, 2016
@daviddias daviddias deleted the feat/awesome-dag-pb branch November 24, 2016 12:33
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants