Wrap zip lib with simple promise-based API (fixes #16) #20
Conversation
muffinresearch
changed the title
| this.zipLib.open(this.filename, {autoClose: false}, | ||
| (err, zipfile) => { | ||
| if (err) { | ||
| reject(err); |
There was a problem hiding this comment.
Should we return reject(err) here? Just so we don't bother to execute the other stuff.
There was a problem hiding this comment.
Yes, we should. Good catch!
| throw new Error('path does not exist in metadata', path); | ||
| } | ||
|
|
||
| var entry = this.metadata[path]; |
There was a problem hiding this comment.
entry is only used once on the very next line. Is this here to cut down on line length on the next line?
There was a problem hiding this comment.
Yeah probably uncessary.
|
Looks good, just wondering about that |
muffinresearch
force-pushed
the
add-xpi-handling
branch
from
2e3b97b
to
86e1f36
Compare
|
@tofumatt ok, I think this is ready for another look. |
| readStream | ||
| .on('readable', function () { | ||
| var chunk; | ||
| while (null !== (chunk = readStream.read())) { |
There was a problem hiding this comment.
Putting the null first feels a bit Pythonic, not used to that in JS.
If this is preferred style I'll just make a note of it and be sure to include it in #21.
There was a problem hiding this comment.
That came from the docs example: https://nodejs.org/api/stream.html#stream_readable_read_size
There was a problem hiding this comment.
Fair enough, seems good 😃
- tofumatt (Sent from mobile)
On 25 Sep 2015, at 12:30, Stuart Colville notifications@github.com wrote:
In tests/test.xpi.js:
- this.openStub.yieldsAsync(null, this.fakeZipFile);
- var rstream = new Readable();
- rstream.push('line one\n');
- rstream.push('line two');
- rstream.push(null);
- this.openReadStreamStub.yields(null, rstream);
- myXpi.getFileAsStream('install.rdf')
.then((readStream) => {var chunks = '';readStream.on('readable', function () {var chunk;while (null !== (chunk = readStream.read())) {—
Reply to this email directly or view it on GitHub.
|
r+, let me know about that |
Wrap zip lib with simple promise-based API (fixes #16)
No description provided.