Skip to content

Conversation

@megoth
Copy link
Contributor

@megoth megoth commented Nov 27, 2019

Upgrading the default version we're using to build this project from 8.16 to 12.7.

Also adding 8.10 as the lowest version we'll currently support (works with ESLint) so that we know when it breaks (and then fix it or reevaluate which versions we'll support).

package-lock.json is simply the new generated file after upgrading to 12.7.

Copy link
Contributor

@Vinnl Vinnl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe give Tim a heads-up after merging this, I don't think he uses nvm?

@megoth
Copy link
Contributor Author

megoth commented Nov 27, 2019

It should work with any versions newer than 8.10, so I think he's good, but yeah, I'll check with him ^_^

@Vinnl
Copy link
Contributor

Vinnl commented Nov 27, 2019

@megoth Yeah it should work, but with the newer version you'd also get a newer version of npm, and in between the lockfile format changed. So if Tim would continue to use the old version of npm, we'd get a new lockfile with many changes every time someone runs npm install with a different version than the last time the file was generated :)

@RubenVerborgh
Copy link
Contributor

Tim needs to upgrade 😄

@megoth Is DEFAULT_NODE_VERSION still used anywhere? Looks like a leftover from older build scripts.

@megoth
Copy link
Contributor Author

megoth commented Nov 27, 2019

@megoth Is DEFAULT_NODE_VERSION still used anywhere? Looks like a leftover from older build scripts.

I'll double-check, gotta admit it was shamelessly copied from https://github.com/rdfjs/N3.js/blob/master/.travis.yml#L3-L7 without much though -_-

@megoth
Copy link
Contributor Author

megoth commented Nov 27, 2019

I can't find any information for why it should be there, so I'll remove it.

@megoth
Copy link
Contributor Author

megoth commented Nov 27, 2019

@RubenVerborgh I was thinking of switching out npm install with npm ci (npm-ci), which means that we must test with Node v8.12.0 (npm v6.4.1) and Node v10.3.0 (nvm v6.1.0).

I think it would be good to have a clean install and be 100% sure that package-lock.json describes what dependencies to use.

Do you have thoughts on this?

@RubenVerborgh
Copy link
Contributor

Do you have thoughts on this?

npm ci.

@megoth megoth merged commit 352cf11 into master Nov 28, 2019
@megoth megoth deleted the upgrade-node branch November 28, 2019 16:28
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

Successfully merging this pull request may close these issues.

5 participants