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.

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

hubot package installs with .node-version breaking nodenv #977

Closed
jasonkarns opened this issue Jun 10, 2015 · 14 comments
Closed

hubot package installs with .node-version breaking nodenv #977

jasonkarns opened this issue Jun 10, 2015 · 14 comments
Assignees

Comments

@jasonkarns
Copy link
Contributor

The npm package is published with a .node-version file. I can't seem to find when, how or where this file is generated, but I humbly submit that it should not be included in the package (nor the git repo).

As has been discussed in the ruby community numerous times, the .node-version file should not be shared as it requires an overly strict version specifier.

Does anyone know where this file is coming from before I go further in advocating for its removal?

@technicalpickles
Copy link
Member

I don't see it in git:

$ git rm .node-version
fatal: pathspec '.node-version' did not match any files

I generated a new bot and checked it's using 2.13.1. I did the release over the weekend, and I don't have that file locally, so I'm not sure where it's coming from.

Can you demonstrate how this is breaking nodenv? I use it myself, and have not seen a problem, since it'd be inside a projects's node_modules, not it's parent.

@jasonkarns
Copy link
Contributor Author

I'm terribly confused where it's coming from:

$ npm i hubot
$ ls -al node_modules/hubot/.node-version 
-rw-r--r--  1 jasonkarns  wheel     6B Nov 21  2014 node_modules/hubot/.node-version

(modified time of Nov 21, 2014? weird)

I verified it's in the actual package from npm: http://registry.npmjs.org/hubot/-/hubot-2.13.1.tgz

It breaks on my nodenv because the contents of the node-version aren't a valid version name (v0.10). nodenv version specifiers don't have the v prefix, and include the full major.minor.patch version number.

$ bin/hubot 
/Users/jasonkarns/Projects/td/doubot/node_modules/hubot/node_modules/.bin
nodenv: version `v0.10' is not installed

@technicalpickles
Copy link
Member

Did you generate a hubot with yo hubot? Try running bin/hubot from /Users/jasonkarns/Projects/td/doubot instead.

@technicalpickles
Copy link
Member

Also, turns out there are multiple nodenvs: https://github.com/wfarr/nodenv

@jasonkarns
Copy link
Contributor Author

Try running bin/hubot from /Users/jasonkarns/Projects/td/doubot instead.

That is where I'm running bin/hubot from:

$ pwd
/Users/jasonkarns/Projects/td/doubot
$ bin/hubot 
/Users/jasonkarns/Projects/td/doubot/node_modules/hubot/node_modules/.bin
nodenv: version `v0.10' is not installed

Also, turns out there are multiple nodenvs: https://github.com/wfarr/nodenv

Yep. I use @OiNutter's nodenv which is a direct port of rbenv with virtually no modifications. (I'm one of the maintainers)

@tombell
Copy link
Contributor

tombell commented Jun 11, 2015

If you have a .node-version file in the directory, it will be picked up by npm as it uses its own .npmignore file, and delegates to .gitignore when it doesn't exist. .node-version isn't actually in the .gitignore so will be included when hubot is pushed to npmjs.org if one exists.

@jasonkarns
Copy link
Contributor Author

So we need to add .node-version to the .gitignore and need to publish a new version to npm?

@tombell
Copy link
Contributor

tombell commented Jun 12, 2015

Yup, that would be a solution.

@michaelansel
Copy link
Collaborator

@jasonkarns, can you prep a PR and I'll merge and cut a release tomorrow?

@jasonkarns
Copy link
Contributor Author

@michaelansel #980

@michaelansel michaelansel self-assigned this Jun 12, 2015
@technicalpickles
Copy link
Member

We could also git ignore .node-version. Probably worth adding sanity check
to release to make sure there are random files in unchecked into git that'd
accidentally be included.

On Thursday, June 11, 2015, Jason Karns [email protected] wrote:

@michaelansel https://github.com/michaelansel #980
#980


Reply to this email directly or view it on GitHub
#977 (comment).

@jasonkarns
Copy link
Contributor Author

#980 does add .node-version to gitignore. If we were to create an .npmignore, then npm would no longer use .gitignore, and we would have to duplicate ignore entries to both files.

@technicalpickles
Copy link
Member

@jasonkarns missed the PR for my last comment, thanks!

@technicalpickles
Copy link
Member

Fixed in #980 and the next release shouldn't have the same problem.

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

No branches or pull requests

4 participants