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

fix(cli): add output for cli init #475

Merged
merged 1 commit into from
Sep 12, 2016
Merged

Conversation

victorb
Copy link
Member

@victorb victorb commented Sep 10, 2016

Shows exactly same information as go-ipfs except for change of the cmd +
cat so ipfs cat turns into jsipfs files cat instead. Ref: issue #286

@victorb
Copy link
Member Author

victorb commented Sep 10, 2016

This is the output of init with this change:

$ export IPFS_PATH=~/.ipfs-js
$ node src/cli/bin.js init 
initializing ipfs node at /home/victor/.ipfs-js
generating 2048-bit RSA keypair...done
peer identity: QmP7HvVg8wJV1YvbnmFsnxAP5ULyZaeCxhJx12kizyybfL
to get started, enter:

     jsipfs files cat /ipfs/QmegvLXxpVKiZ4b57Xs1syfBVRd8CbucVHAp7KpLQdGieC/readme

@victorb
Copy link
Member Author

victorb commented Sep 10, 2016

Not sure why Travis is failing here, working fine in CircleCI

@daviddias
Copy link
Member

@victorbjelkholm while you are this, can you alias ipfs files {cat, add, get, ls} to ipfs {cat, add, get, ls}, so that we can use the Sharness and Benchmark tests?

process.stdout.write(msg)
}
}
}
Copy link
Member

Choose a reason for hiding this comment

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

Isn't this just a console.log? All invisible logs should be through debug (http://npmjs.org/debug)

Copy link
Member Author

@victorb victorb Sep 12, 2016

Choose a reason for hiding this comment

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

Almost, it's console log with the ability of deciding if next message should go on a new line or not. Necessary for the generating keys message, which prints "done" after it's done.

@victorb
Copy link
Member Author

victorb commented Sep 12, 2016

@diasdavid Sure, I'll create the aliases in a different PR though.

@@ -49,3 +49,15 @@ exports.getIPFS = (callback) => {
exports.getRepoPath = () => {
return process.env.IPFS_PATH || os.homedir() + '/.ipfs'
}

exports.createLogger = (visible) => {
return (msg, newline = true) => {
Copy link
Member

Choose a reason for hiding this comment

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

This will fail on node 4 and 5, (default arguments are only supported in node 6)

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh, I was sure that js-ipfs was compiled, even when used from node. Is that not correct?

Copy link
Member

Choose a reason for hiding this comment

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

We do provide a compiled version when publishing do npm, but the tests run on node against the original source atm.

Copy link
Member Author

Choose a reason for hiding this comment

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

Shouldn't the tests run against the compiled version that we distribute rather than the source we develop with? Anyways, I'll fix this but worth thinking about.

Copy link
Member

Choose a reason for hiding this comment

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

If you npm install it, you can use both versions.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah, but require('ipfs') would use the compiled version right?

Copy link
Member

Choose a reason for hiding this comment

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

Yes. And the tests in the browser do run against the compiled version

Shows exactly same information as go-ipfs except for change of the cmd +
cat so `ipfs cat` turns into `jsipfs files cat` instead. Ref: issue #286
@daviddias daviddias merged commit 37db11e into master Sep 12, 2016
@daviddias daviddias deleted the issue-286-init-output branch September 12, 2016 19:44
@daviddias daviddias removed the status/in-progress In progress label Sep 12, 2016
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