search

nfriedly / node-bestzip

Provides a `bestzip` command that uses the system `zip` if avaliable, and a Node.js implimentation otherwise.
MIT License
80 stars 16 forks source link

Use newest archiver (3.0.0), drop archiver.bulk() #21

Closed bdelee-zz closed 5 years ago

nfriedly commented 5 years ago

Hey, thank you!

This is definitely the right direction to move in, and I think it's probably a safe change, but since I ended up breaking things the last time I merged in a change that I thought was safe, I'm going to ask a couple of people to test, and then I'll merge it in early next week if I don't hear any issues.

(Also, sorry for the slow response, I was out yesterday)

bdelee-zz commented 5 years ago

Yeah, of course. I feel bad that I pushed a PR that broke stuff. I didn't realize the unit tests didn't hit coverage, I should have checked that myself before pushing it. Hopefully this is a more stable, consistent solution anyway. Using Archiver.directory() for directories and Archiver.file() for files seems a little more intuitive (to me, at least).

nfriedly commented 5 years ago

@jeswinsimon, @aramirez-asuresoftware, & @Lampei: Can you guys give this a shot and let me know if it works as expected?

npm install --no-save https://github.com/bdelee/node-bestzip.git

Also, @Lampei, sorry I never noticed #10 / #11 before; can you let me know if this fixes that issue?

Relates to #10, #11, #18, #19

nfriedly commented 5 years ago

Don't worry about it too much @bdelee, that was more my fault than anyone's. I didn't even realize that was you ;)

nfriedly commented 5 years ago

I just released a v2.0.0 that includes this change as well as quite a few other changes (and tests!) to normalize things and ensure equal results across different operating systems.

jeswinsimon commented 5 years ago

This is working fine. Thanks!