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

WIP cat interface core #280

Closed
wants to merge 1 commit into from
Closed

Conversation

nginnever
Copy link
Contributor

This begins the js-ipfs-api cat command using the interace-core tests shared with the js-ipfs core tests.

This needs the completition of the cat interface-core PR

})
})
})
test.files(common)
Copy link
Contributor

Choose a reason for hiding this comment

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

Note to self: once I get in the add test PR, rename this file to files-spec.js, so we don't run the interface files tests twice.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thought of this as well, but calling it file.spec.js since files.spec.js is taken by the mfs tests

Copy link
Contributor

Choose a reason for hiding this comment

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

Wasn't there an issue somewhere about explicitly renaming that to mfs across js-ipfs? Having trouble finding that..

@hackergrrl
Copy link
Contributor

hackergrrl commented May 22, 2016

🐴 LGTM

@@ -4,6 +4,7 @@ const argCommand = require('../cmd-helpers').argCommand

module.exports = (send) => {
return {
cat: argCommand(send, 'cat'),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is a mistake, should not be here

return cat
}

function cleanMultihash (multihash) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is defined in src/api/object.js too. Could you toss it into a module in src/?

@nginnever
Copy link
Contributor Author

closing in favor of #292 where others can edit.

@nginnever nginnever closed this Jun 5, 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.

3 participants