added buffer importer #6
Conversation
| @@ -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) | |||
There was a problem hiding this comment.
thank you for catching this one! :D
|
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) |
|
@nginnever what's missing on this one for it to be merged? |
|
@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' | |||
|
@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" |
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