Skip to content

🏗 Update node major version in package.json to LTS#19159

Merged
rsimha merged 1 commit intoampproject:masterfrom
rsimha:2018-11-06-NodeMinVer
Nov 12, 2018
Merged

🏗 Update node major version in package.json to LTS#19159
rsimha merged 1 commit intoampproject:masterfrom
rsimha:2018-11-06-NodeMinVer

Conversation

@rsimha
Copy link
Copy Markdown
Contributor

@rsimha rsimha commented Nov 6, 2018

We have a new LTS version of node, so we must update the min version in package.json and .travis.yml.

In .travis.yml, we can simply specify that the version to use is lts/*. However, in package.json, we must specify the actual LTS major version, since npm doesn't support a wildcard for LTS. See https://docs.npmjs.com/files/package.json#engines.

This is the fix discussed in #19050 (comment)

Follow up to #19046 and #19051
Fixes #19050

Copy link
Copy Markdown
Contributor

@cvializ cvializ left a comment

Choose a reason for hiding this comment

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

I think we want to wait on the discussion, since this will break people who haven't upgraded from 8.x.x to 10.x.x (see #19050 (comment))

@rsimha
Copy link
Copy Markdown
Contributor Author

rsimha commented Nov 6, 2018

@cvializ SGTM. Thanks for the pointer.

Copy link
Copy Markdown
Contributor

@jridgewell jridgewell left a comment

Choose a reason for hiding this comment

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

Pending #19050.

@rsimha
Copy link
Copy Markdown
Contributor Author

rsimha commented Nov 12, 2018

@cvializ @jridgewell Resulting plan from the discussion: We use the latest LTS on Travis, and allow all active LTS version via package.json: See #19050 (comment)

PTAL.

@rsimha rsimha merged commit b8445ff into ampproject:master Nov 12, 2018
@rsimha rsimha deleted the 2018-11-06-NodeMinVer branch November 12, 2018 18:34
@cvializ
Copy link
Copy Markdown
Contributor

cvializ commented Nov 12, 2018

This also needs to be added to the validator package.jsons, correct?

"node": "^8.0.0"

"node": "^8.0.0"

"node": "^8.0.0"

@rsimha
Copy link
Copy Markdown
Contributor Author

rsimha commented Nov 12, 2018

@cvializ Good point. Coming up.

@cvializ
Copy link
Copy Markdown
Contributor

cvializ commented Nov 12, 2018

sg, the build will be red until that change is made. #19051 had to undo this same change last time the node version was updated because of build breakage

@rsimha
Copy link
Copy Markdown
Contributor Author

rsimha commented Nov 12, 2018

Fixed with #19264. Thanks for the heads up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Discussion to avoid node version incompatibility with LTSs

5 participants