Skip to content
This repository has been archived by the owner on Feb 12, 2024. It is now read-only.

Add a few examples to README.md #534

Closed
wants to merge 8 commits into from
Closed

Add a few examples to README.md #534

wants to merge 8 commits into from

Conversation

SidHarder
Copy link
Member

No description provided.

> **Will come soon**
```js
var fs = require('fs');
var promisify = require('promisify-es6')
Copy link
Contributor

Choose a reason for hiding this comment

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

The variable promisify does not seem to be used anywhere below in your examples.

(Should you decide to delete this line?)

Copy link
Member Author

Choose a reason for hiding this comment

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

I will remove that line. Thank you.

stream.pipe(process.stdout, { end : false });
});
}
```
Copy link
Member

Choose a reason for hiding this comment

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

please change the code style to be consistent with our code base, i.e standard

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for your feedback. I will read the standard and apply.

Copy link
Member

Choose a reason for hiding this comment

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

Thank you :)

}

function startDaemon() {
node.goOnline(function(msg) {});
Copy link
Member

Choose a reason for hiding this comment

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

This should probably call addFile in the callback to trigger the interaction once the node is online.

Copy link
Member Author

Choose a reason for hiding this comment

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

I will fix this up.

Copy link
Member Author

Choose a reason for hiding this comment

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

Is the idea that you would go online do some work and then go offline.

Copy link
Member

Choose a reason for hiding this comment

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

That was my thought at least, did you have something else in mind?

Copy link
Member

@victorb victorb left a comment

Choose a reason for hiding this comment

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

Looks good to me, but I would change one thing. Right now, the example is not actually doing anything but setting up some functions for future need. I think it would be useful to have the whole dance of init -> add file to IPFS, get hash -> get file via hash and print it out.

@SidHarder
Copy link
Member Author

I'll will make it dance.

@SidHarder
Copy link
Member Author

I'm not able to get Node.id() to work. Am I missing something? The promise doesn't ever return, It appears to hang.

node.id().then(function(data) {
conosle.log(data)
})

@SidHarder
Copy link
Member Author

Let me know what you guys think about this example!

@victorb
Copy link
Member

victorb commented Oct 31, 2016

@SidHarder looks great! Thanks a lot for adding this. Got two minor points.

  • Probably want to change the default path to the repo, instead of using ~/.ipfs
  • Seems you're mixing callbacks and promises. We should just use callbacks instead.

@SidHarder
Copy link
Member Author

@victorbjelkholm I would also prefer to use callbacks but for example when I use this:

node.files.add(readStream, function(data) {
console.log(data)
})

There is no data. I don't seem to get the hash from the call back.

When I do:

node.files.add(readStream).then(function (data) {
console.log(data)
})

I get the hash back and can proceed. I am missing something? This is what led me to mix callback with promises.

Thanks for your help.

@Mithgol
Copy link
Contributor

Mithgol commented Nov 1, 2016

@SidHarder Node.js callbacks tend to have the signature (error, something) — you're getting “no error” (instead of “no data”) as your function's first argument.

@SidHarder
Copy link
Member Author

@Mithgol I forgot, error first. Thanks.

@SidHarder
Copy link
Member Author

@victorbjelkholm I'm afraid I don't get your point on this "Probably want to change the default path to the repo, instead of using ~/.ipfs".

@SidHarder
Copy link
Member Author

I see that someone made some changes to the readme.md and did a much better job. This pr can be closed. Thanks for everyone's help.

@daviddias
Copy link
Member

daviddias commented Nov 7, 2016

Oh, apologies @SidHarder, should have had continued on top of your PR, but ended up just completing the Readme as I was updating it with the awesome IPLD PR as I wanted to open this discussion #556

Nevertheless, your example has way more information, could you PR the example as working code into the examples folder?

@victorb
Copy link
Member

victorb commented Nov 8, 2016

Closing in favor of #576

@victorb victorb closed this Nov 8, 2016
MicrowaveDev pushed a commit to galtproject/js-ipfs that referenced this pull request May 22, 2020
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.

5 participants