Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix #79 outdated dependencies in unzip2 #463

Closed
wants to merge 1 commit into from
Closed

fix #79 outdated dependencies in unzip2 #463

wants to merge 1 commit into from

Conversation

jsamr
Copy link

@jsamr jsamr commented Dec 18, 2017

This is a workaround unmaintained unzip2 package.
Upstream custom package sveinburne/unzip2 tests passed with upgraded dependencies.
Passed this package tests too.

New dependency graph for unzip2:

{
    "binary": "^0.3.0",
    "fstream": "^1.0.11",
    "match-stream": "0.0.2",
    "pullstream": "^0.4.1",
    "readable-stream": "^2.3.3",
    "setimmediate": "^1.0.5"
},

@guyonroche
Copy link
Collaborator

@sveinburne I'm not so keen on github dependencies. Will you be supporting unzip2's replacement, if so, could you publish to npm?

@jsamr
Copy link
Author

jsamr commented Jan 16, 2018

@guyonroche Not planning on supporting a replacement, sorry :-(

@kachkaev kachkaev mentioned this pull request Feb 20, 2018
Merged
@kachkaev
Copy link

kachkaev commented Feb 20, 2018 •
edited
Loading

If glebdmitriew/node-unzip-2#21 is merged any time soon, there won't be a need for this PR 🎉 🤞

PS: I'm also here from this yarn warning:

warning exceljs > unzip2 > fstream > graceful-fs@3.0.11: please upgrade to graceful-fs 4 for compatibility with current and future versions of Node.js

@kachkaev
Copy link

kachkaev commented Feb 20, 2018 •
edited
Loading

Good news – there is a new release on npm!

It somehow happened that unzip2's repo on github (https://github.com/glebdmitriew/node-unzip-2) was published to npm by someone else, not the author. See https://www.npmjs.com/package/unzip2.

A couple of hours ago the author of the github repo published the library as https://www.npmjs.com/package/node-unzip-2 and repointed README to node-unzip-2. Looks like node-unzip-2 has existed for a while, but was not mentioned from the github repo.

So here's what should work:

npm rm unzip2
npm install node-unzip-2

require / import statements will need an update too.

@kachkaev
Copy link

@jsamr would you be happy to submit a PR for that?

@guyonroche
Copy link
Collaborator

@kachkaev - massive thanks for doing all this legwork!
If that's all it takes (I've just run the tests and it seems so) then I can push your suggested change now

@kachkaev
Copy link

That'd be great! Thanks for this truly awesome library!

@guyonroche
Copy link
Collaborator

Closing as fixed in master. Released as 0.8.3

@guyonroche guyonroche closed this Feb 20, 2018
@kachkaev
Copy link

kachkaev commented Feb 20, 2018 •
edited
Loading

I confirm no more warnings from exceljs after upgrading! 🎉

@jsamr thanks for your efforts to resolve this! I probably would not start digging though github repos and npm packages if you did not submit this PR!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@jsamr @guyonroche @kachkaev