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

Update Kit to use latest active LTS Node.js version 12.x #840

Merged
merged 5 commits into from
Nov 29, 2019

Conversation

NickColley
Copy link
Contributor

@NickColley NickColley commented Nov 21, 2019

Since the Node.js website only shows 12 it makes sense for us to support the version new users will be installing.

Carbon LTS release (8.x) is end of life December I have dropped this to focus on only the LTS versions that will be maintained, this will cut down on build times, but the kit should still work just will not be tested by CI.

I have also opened pull requests to the govuk-design-system and govuk-frontend repositories to keep everything aligned.

Closes #828

@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-prototype-kit-pr-840 November 21, 2019 11:19 Inactive
@NickColley NickColley force-pushed the update-kit-to-focus-on-latest-lts-release-12 branch from a9d5643 to 9ec2093 Compare November 21, 2019 11:22
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-prototype-kit-pr-840 November 21, 2019 11:22 Inactive
@NickColley NickColley force-pushed the update-kit-to-focus-on-latest-lts-release-12 branch from 9ec2093 to 4e7605e Compare November 21, 2019 11:46
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-prototype-kit-pr-840 November 21, 2019 11:47 Inactive
@NickColley NickColley marked this pull request as ready for review November 21, 2019 12:04
@NickColley NickColley force-pushed the update-kit-to-focus-on-latest-lts-release-12 branch from 4e7605e to 21f9210 Compare November 21, 2019 12:09
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-prototype-kit-pr-840 November 21, 2019 12:09 Inactive
@NickColley
Copy link
Contributor Author

I've put blocked on this as I believe that we should get everything to 12 at the same time if possible to avoid issues when developing between these projects but let's discuss the issues in the GOV.UK Design System repository and decide what to do.

@NickColley
Copy link
Contributor Author

NickColley commented Nov 21, 2019

Spoke to @36degrees, we're going to run different versions for now, see full statement here: alphagov/govuk-design-system#1091 (comment)

Copy link

@amyhupe amyhupe left a comment

Choose a reason for hiding this comment

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

Looks good to me

.travis.yml Outdated
- "10.15.1"
- "8"
- "10.17.0" # lts/dubnium
- "12.13.1" # lts/erbium
Copy link
Contributor

Choose a reason for hiding this comment

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

can we use "10" and "12" so that we build against the latest minor/patch releases?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it's better to have it pinned so that our build is stable, otherwise it'll update over time and could start failing?

Copy link
Contributor

Choose a reason for hiding this comment

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

if it fails that might be good to know - it could be failing for users downloading and using that latest version of Node

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm trying to keep our development environments consistent with CI since we don't often work on Kit features, I'd prefer @36degrees make the decision here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The other repositories are not run by our users so I think let's make an exception here and take the risk of it blowing up now and then.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's better to know in the (hopefully unlikely) case that a new minor version causes issues with the kit.

package.json Outdated
@@ -4,7 +4,7 @@
"version": "9.4.0",
"private": true,
"engines": {
"node": "^10.0.0"
"node": "12.13.1"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"node": "12.13.1"
"node": "^12.0.0"

?

@joelanman
Copy link
Contributor

Just to note I don't think this closes #828 on its own - the documentation still needs updating to not tell people to get Node 10 (currently I think the 'closes 828' comment might automatically close 828 if this is merged, maybe?)

@NickColley NickColley changed the title Update support matrix to include most recent LTS releases Update Kit to use latest active LTS Node.js version 12.x Nov 22, 2019
@NickColley NickColley force-pushed the update-kit-to-focus-on-latest-lts-release-12 branch from 21f9210 to adf2b54 Compare November 22, 2019 10:23
@36degrees
Copy link
Contributor

Just to note I don't think this closes #828 on its own - the documentation still needs updating to not tell people to get Node 10 (currently I think the 'closes 828' comment might automatically close 828 if this is merged, maybe?)

@joelanman this PR updates the documentation that I'm aware of that mentions any specific version of node – is there something we've missed?

@joelanman
Copy link
Contributor

@36degrees the documentation changes were made after my comment, looks all good now!

@NickColley NickColley merged commit 318f90e into master Nov 29, 2019
@NickColley NickColley deleted the update-kit-to-focus-on-latest-lts-release-12 branch November 29, 2019 10:52
@36degrees 36degrees modified the milestone: v9.5.0 Jan 10, 2020
@hannalaakso hannalaakso mentioned this pull request Jan 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

Install guide not working as you can't easily get Node 10
5 participants