-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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.
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
Fixed GitHub dependencies #520
base: master
Are you sure you want to change the base?
Conversation
Optimized the process of installing dependencies from GitHub. It now uses the tarball of the CURRENT commit in master. Previously it was cloning the entire repository which wasn't needed and when it comes to express and socket.io, things get pretty big. This also fixes some issues that `npm` on Windows and other OSes have with malformed git addresses.
This one fixes the issue. Good job! |
@popcorn-time Can we get this merged? I can't do anything at the moment unless I manually edit the packages.json file so I can pull in dependencies. |
I think we also need to fix github URLs which I believe should be in the following format "adm-zip" : "git+https://github.com/gpt-modules/adm-zip.git", instead of just "adm-zip" : "https://github.com/gpt-modules/adm-zip/tarball/master", otherwise I believe npm install would not work correctly and show errors. |
@simofacc No. You misunderstand how my pull request works Under normal circumstances you would be correct, however The method I have used actually uses |
Ok that is good then. |
@simofacc Yea. I would have much rathered to have used the Git URLs however on Windows I just could not for the life of me get it to work. Browsing over the NPM source code it actually looks like using the GitHub feature of NPM (eg. |
@jduncanator You should use git+https://, not https:// |
@Sekhmet Did you even read my above comment? |
Optimized the process of installing dependencies from GitHub. It now uses the tarball of the CURRENT commit in master. Previously it was cloning the entire repository which wasn't needed and when it comes to express and socket.io, things get pretty big. This also fixes some issues that
npm
on Windows and other OSes have with malformed git addresses.