remy / inliner

Node utility to inline images, CSS and JavaScript for a web page - useful for mobile sites
MIT License
1.1k stars 165 forks source link

Add missing version property in package.json #139

Closed pinkhominid closed 7 years ago

pinkhominid commented 7 years ago

NPM states that version is a required property in package.json. I'm having trouble npm installing a forked branch because this is missing.

npm i git://github.com/pinkhominid/inliner.git#5f8f1e89412a7ac427398a07d552fd358a49e9bd npm ERR! addLocal Could not install /var/folders/5w/7lzcyvqs3ys1j2_syyfh21c00000gr/T/npm-5466-25b74c34/git-cache-14a0e5c9/5f8f1e89412a7ac427398a07d552fd358a49e9bd npm ERR! Darwin 14.5.0 npm ERR! argv "/usr/local/bin/node" "/Users/john/.node/bin/npm" "i" "git://github.com/pinkhominid/inliner.git#5f8f1e89412a7ac427398a07d552fd358a49e9bd" npm ERR! node v7.6.0 npm ERR! npm v4.3.0

npm ERR! No version provided in package.json npm ERR! npm ERR! If you need help, you may report this error at: npm ERR! https://github.com/npm/npm/issues

npm ERR! Please include the following file with any support request: npm ERR! /Users/john/.npm/_logs/2017-04-04T20_34_25_666Z-debug.log

remy commented 7 years ago

npm isn't missing the version in the package.json: http://registry.npmjs.org/inliner/latest

You're referring to the package.json in this repo, which is not equal to the package.json that's submitted to npm. I use semantic-release to automatically manage this, so it's not a problem.

pinkhominid commented 7 years ago

So that allows me to install from the npm registry but I cannot install direct from github.

npm i git://github.com/remy/inliner.git

Fails with npm ERR! No version provided in package.json

The reason I care is that I need to npm install from my fork since the pull request is sitting in limbo. I think this is a good thing to allow as people can continue to use your project with custom fixes whether they get merged or not.

The alternative is I have to publish my fork to the npm registry which kind of sucks for all involved.

remy commented 7 years ago

Publishing a fork does make sense to me if that's what you actually need.

As for the PR, it was waiting on this comment: https://github.com/remy/inliner/pull/135#pullrequestreview-29428136 (I do appreciate this isn't your naming convention, and doesn't quite wash the sentiment out of the code, but if you make the change, and the commit style is right, I can merge and it'll auto deploy).