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

added buffer importer #6

Merged
merged 1 commit into from
Mar 21, 2016
Merged

added buffer importer #6

merged 1 commit into from
Mar 21, 2016

Conversation

nginnever
Copy link
Contributor

basically this is the same as importing from a file but under the assumption that the importer should take a prebuffered file in memory up to the js max of 1gb.

also replaced the out of bounds parameters in fixedSizeChunker for chunks larger than the fixed size.

also added condition at the top of tests to look for empty dir needed that github didn't push for testing nested dirs

@daviddias daviddias self-assigned this Feb 22, 2016
@@ -23,7 +23,7 @@ function FixedSizeChunker (size) {
var chunk = new Buffer(size, 'binary')
var newBuf = new Buffer(buf.length - size, 'binary')
buf.copy(chunk, 0, 0, size)
buf.copy(newBuf, 0, size - 1, buf.length - size)
buf.copy(newBuf, 0, size, buf.length)
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 for catching this one! :D

@daviddias
Copy link
Contributor

LGTM (other than the small things) , thank you @nginnever :)

If you can, let's get this module prepared to run in the browser next (standard webpack + karma testing setup)

@daviddias
Copy link
Contributor

@nginnever what's missing on this one for it to be merged?

@nginnever
Copy link
Contributor Author

@diasdavid I'm about to push some commits I worked on over the weekend after the help I got from @dignifiedquire

So I have all the fixed-sized-chunker tests passing and will skip the web tests for file/dir importer and only test the buffer importer. I will get the buffer tests running today. I was thinking about adding the stream importer and tests in this PR but I still have a couple of questions about how we will flag the stream as a file or a dir or the best approach to this. So maybe I should make a separate PR for stream importer.

After that I just need to standard the code up and have you take a look.

Thanks!

@@ -0,0 +1,56 @@
'use strict'
Copy link
Contributor

Choose a reason for hiding this comment

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

is this needed?

@daviddias
Copy link
Contributor

@nginnever CR'ed. Overall LGTM, I'll wait for travis to merge, meanwhile, I left some little nitpicks :)


language: node_js
node_js:
- "4.0"
Copy link
Contributor

Choose a reason for hiding this comment

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

let's do:

  • '4'
  • '5'

daviddias added a commit that referenced this pull request Mar 21, 2016
@daviddias daviddias merged commit d1a9ddd into ipfs-inactive:master Mar 21, 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.

2 participants