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

refactor: replace wreck with raw request #414

Merged
merged 3 commits into from
Nov 8, 2016
Merged

refactor: replace wreck with raw request #414

merged 3 commits into from
Nov 8, 2016

Conversation

dignifiedquire
Copy link
Contributor

No description provided.

expect(result.length).to.equal(1)
done()
})
})
Copy link
Contributor

Choose a reason for hiding this comment

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

nice :)

function parseRaw (res, cb) {
res
.once('error', cb)
.pipe(concat((data) => cb(null, data)))
Copy link
Contributor

Choose a reason for hiding this comment

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

why not use bl?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because concat does the right thing for streams, arrays and buffers

Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you :) Will remind me of that the next time I need to do this

Copy link
Contributor

@daviddias daviddias left a comment

Choose a reason for hiding this comment

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

LGTM. Does this mean that browserify is now golden?

})
})
}

function display (hash) {
ipfs.cat(hash, function (err, res) {
ipfs.cat(hash, {buffer: true}, function (err, res) {
Copy link
Contributor

Choose a reason for hiding this comment

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

It needs to be buffered?

Copy link
Contributor

Choose a reason for hiding this comment

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

I still think this buffer: true is one of the most misleading features. Can you add a comment that says:
"Buffer the whole response instead of streaming it"

@@ -4,15 +4,13 @@
"description": "",
"main": "index.js",
"scripts": {
"start": "browserify -t brfs index.js > bundle.js && http-server -a 127.0.0.1 -p 8888"
"start": "browserify index.js > bundle.js && http-server -a 127.0.0.1 -p 8888"
Copy link
Contributor

Choose a reason for hiding this comment

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

<3

@daviddias
Copy link
Contributor

btw, you missed the chance of calling this brach 'reeeeekt!' ahah :D

@daviddias daviddias merged commit f46cdc5 into master Nov 8, 2016
@daviddias daviddias deleted the wreck branch November 8, 2016 12:16
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.

2 participants