Skip to content

feat: update supported Node.js versions #4675

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

Merged
merged 1 commit into from
Jun 4, 2019

Conversation

rosen-vladimirov
Copy link
Contributor

Update supported Node.js versions according to https://github.com/nodejs/Release

  • Stop support for Node.js 6 and 7 - CLI will not allow execution of any command when this Node.js is used.
  • Deprecate support for Node.js 8 and 9 - CLI will print warning on each command when any of those versions is used
  • Add official support for Node.js 12 - CLI will not print warning for this version as it is officially supported now.

Also fix the check of getNodeWarning method which was returning warning only the first time when it is called.

PR Checklist

What is the current behavior?

CLI prints deprecation warning when Node.js 6.x.x or 7.x.x is used.
CLI stops execution when Node.js < 6 is used.
CLI prints warning when Node.js 12 is used.

What is the new behavior?

CLI prints deprecation warning when Node.js 8.x.x or 9.x.x is used.
CLI stops execution when Node.js < 8 is used.
CLI does not print warning when Node.js 12 is used

Implements:

Update supported Node.js versions according to https://github.com/nodejs/Release
- Stop support for Node.js 6 and 7 - CLI will not allow execution of any command when this Node.js is used.
- Deprecate support for Node.js 8 and 9 - CLI will print warning on each command when any of those versions is used
- Add official support for Node.js 12 - CLI will not print warning for this version as it is officially supported now.

Also fix the check of `getNodeWarning` method which was returning warning only the first time when it is called.
@rosen-vladimirov rosen-vladimirov added this to the 6.0.0 milestone Jun 4, 2019
@rosen-vladimirov rosen-vladimirov self-assigned this Jun 4, 2019
@cla-bot cla-bot bot added the cla: yes label Jun 4, 2019
@rosen-vladimirov
Copy link
Contributor Author

test cli-smoke

}

return nodeWarn;
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe you can return a deep copy, so that consumers can't modify the warning. Not a merge blocker.

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 not sure about this, so I'll keep it this way for the moment and I'll keep it in mind in case such issue occurs in the future. Thanks for the suggestion.

@rosen-vladimirov rosen-vladimirov merged commit c9098bb into master Jun 4, 2019
@rosen-vladimirov rosen-vladimirov deleted the vladimirov/node-versions branch June 4, 2019 09:47
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.

2 participants