search

probot / template

Template for new Probot apps
https://probot.github.io/docs/development/#generating-a-new-app
ISC License
51 stars 166 forks source link

Pin node to version 8.10.0 in probot #45

Closed koddsson closed 6 years ago

koddsson commented 6 years ago

This PR makes sure that people that generate their probot using create-probot-app are always running node 8.10.0. The node package installs a node binary in node_modules/ which makes sure that probot will always run with that version when running through npm commands.

📚 Documentation

https://www.npmjs.com/package/node

koddsson commented 6 years ago

I didn't know that was a thing.

So this is a kind of new thing to solve the issue where people would need to look at the engines field in their package.json and then install the correct version of npm. By installing the node binary locally using npm you are guaranteed to run the correct node version when running any kind of npm script command.

Is it considered good practice to declare this as a production dependency?

It is generally considered good practice to mark your library with a engines field so that users and CI can run the package on the correct version.

I usually use the node version manager n to manage my node/npm versions in combination with nodengine to always be running the correct version of node but that requires library authors to declare the engines field. By installing the node package you don't need your users or CI to have more knowledge or tools over just knowing how to install packages.

CI tools like Travis-CI will use nvm to achieve something similar.

The node package is maintained by the same people that maintain npm itself so I am expecting that engines might be deprecated at some point for this method.

bkeepers commented 6 years ago

I think I'm 👎 on listing this as a production dependency until this is standard practice. I haven't seen this used anywhere else, so I'm a little hesitant to add it here.

Do you have any docs or blog posts where people are advocating for using this?

koddsson commented 6 years ago

I think I'm 👎 on listing this as a production dependency until this is standard practice. I haven't seen this used anywhere else, so I'm a little hesitant to add it here.

No worries :)

Do you have any docs or blog posts where people are advocating for using this?

Nope. I haven't seen anyone other then myself use it in the wild.

I'll close it for now :)