Skip to content

Conversation

@bdelee-zz
Copy link
Contributor

No description provided.

@nfriedly
Copy link
Owner

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
Copy link
Contributor Author

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
Copy link
Owner

nfriedly commented Aug 25, 2018

@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
Copy link
Owner

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
Copy link
Owner

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
Copy link

This is working fine. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants