izuzak / atom-pdf-view

Support for viewing PDF files in Atom.
https://atom.io/packages/pdf-view
MIT License
108 stars 30 forks source link

Please bundle pdf.js #24

Closed laktak closed 10 years ago

laktak commented 10 years ago

Could you please bundle pdf.js? We are behind a firewall and cloning from github fails:

Installing “pdf-view@0.10.0” failed.Hide output…
...
npm ERR! git clone git://github.com/mozilla/pdf.js.git github.com[0: 192.30.252.129]: errno=Operation timed out
npm ERR! Error: Command failed: Cloning into bare repository '/Users/laktak/.atom/.node-gyp/.atom/.apm/_git-remotes/git-github-com-mozilla-pdf-js-git-09e1fcd2'...
npm ERR! fatal: unable to connect to github.com:
izuzak commented 10 years ago

@laktak I'll see what I can do -- thanks for pointing this out, hadn't even occurred to me :+1:

Just wondering -- how do you normally clone from GitHub if this doesn't work?

laktak commented 10 years ago

Thanks!

Our firewall is ok with cloning via https: (instead of git:).

izuzak commented 10 years ago

Our firewall is ok with cloning via https: (instead of git:).

Ahhh, right. Could you try something for me, to verify a potential solution:

  1. git clone https://github.com/izuzak/atom-pdf-view/
  2. cd into atom-pdf-view
  3. edit the package.json file so that the pdf.js dependency uses git+https://... and not just git://...
  4. run apm install and let me know if it installs the package properly

That should work as far as I can tell, but just want to double-check before committing changes. Thanks again!

laktak commented 10 years ago

Yep, it works.

But why don't you want to bundle it? The download would be a lot smaller than cloning the whole repo.

izuzak commented 10 years ago

But why don't you want to bundle it? The download would be a lot smaller than cloning the whole repo.

I'm not super-opposed to it and I do agree that the smaller size would be a benefit.

Still, having it this way makes more sense to me at the moment. For example, when I want to update to a new version of pdf.js -- the pull request for that would probably be huge and not be relevant for this project (because it would show the diff between two version of the bundled version). Just bumping the SHA of the commit for which version of pdf.js is cleaner and simpler for humans.

Thanks again for reporting this :bow: