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.

Sign up for GitHub

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

Jump to bottom

Drop node.js v4 support #3364

Merged
merged 1 commit into from
Aug 10, 2018
Merged

Drop node.js v4 support #3364

merged 1 commit into from
Aug 10, 2018

Conversation

outsideris
Copy link
Contributor

Description of the Change

As Node.js announcement and Node.js release schedule, Node.js v4 is end of life April 30th.
Mocha can drop Node.js v4 support.

I checked "Breaking changes between v4 LTS and v6 LTS", but I didn't find anything to change in code level.

Alternate Designs

N/A

Why should this be in core?

It's minimum Node.js version.

Benefits

Mocha doesn't care issues with Node.js v4 anymore.

Possible Drawbacks

N/A

Applicable issues

close #3149

@outsideris outsideris added the area: node.js command-line-or-Node.js-specific label Apr 30, 2018
@coveralls
Copy link

coveralls commented Apr 30, 2018 •
edited
Loading

Coverage Status

Coverage increased (+0.05%) to 90.054% when pulling 549ff78 on outsideris:issue-3149 into 7613521 on mochajs:master.

@boneskull boneskull added this to the v6.0.0 milestone Apr 30, 2018
boneskull
boneskull suggested changes Apr 30, 2018
View reviewed changes
Copy link
Contributor

@boneskull boneskull left a comment

Choose a reason for hiding this comment

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

thanks. see suggestions

docs/index.md Outdated
@@ -92,7 +92,7 @@ or as a development dependency for your project:
$ npm install --save-dev mocha
```

> To install Mocha v3.0.0 or newer with `npm`, you will need `npm` v2.14.2 or newer. Additionally, to run Mocha, you will need Node.js v4 or newer.
> To install Mocha v3.0.0 or newer with `npm`, you will need `npm` v2.14.2 or newer. Additionally, to run Mocha, you will need Node.js v6 or newer.
Copy link
Contributor

Choose a reason for hiding this comment

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

We should just change this whole thing to "Mocha currently requires Node.js v6 or newer".

we do need to get that netlify issue (#3206) addressed though.

Copy link
Contributor Author

@outsideris outsideris May 1, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

I have no experience with Netlify. I will test netify as well.

docs/index.md Outdated
@@ -92,7 +92,7 @@ or as a development dependency for your project:
$ npm install --save-dev mocha
```

> To install Mocha v3.0.0 or newer with `npm`, you will need `npm` v2.14.2 or newer. Additionally, to run Mocha, you will need Node.js v4 or newer.
> To install Mocha v3.0.0 or newer with `npm`, you will need `npm` v2.14.2 or newer. Additionally, to run Mocha, you will need Node.js v6 or newer.

Mocha can also be installed via [Bower](https://bower.io) (`bower install mocha`), and is available at [cdnjs](https://cdnjs.com/libraries/mocha).
Copy link
Contributor

Choose a reason for hiding this comment

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

We should zap the note about Bower while we're 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.

I didn't understand what you mean

Copy link
Contributor

Choose a reason for hiding this comment

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

I mean that we should remove line 97, because new Mocha versions are no longer installable via Bower.

Copy link
Contributor

Choose a reason for hiding this comment

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

I suppose that can be a separate PR.

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 got it. I will make another PR.

boneskull
boneskull suggested changes Apr 30, 2018
View reviewed changes
Copy link
Contributor

@boneskull boneskull left a comment

Choose a reason for hiding this comment

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

thanks. see suggestions

boneskull
boneskull suggested changes Apr 30, 2018
View reviewed changes
Copy link
Contributor

@boneskull boneskull left a comment

Choose a reason for hiding this comment

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

thanks. see suggestions

@boneskull boneskull added the semver-major implementation requires increase of "major" version number; "breaking changes" label Apr 30, 2018
@outsideris outsideris force-pushed the issue-3149 branch 2 times, most recently from 0ca7c30 to 3bbfadd Compare May 1, 2018 06:37
@plroebuck
Copy link
Contributor

For Mocha-6, I think that's fine to drop Node-4. But I think we should maybe have one last Mocha-5.x release that supports Node-4 prior to doing so. Certainly some of problems already fixed could be helpful to those who cannot drop Node-4 yet.

@outsideris
Copy link
Contributor Author

I agreed that dropping v4 in mocha v6. And that's why semver-major label attached here.
Before we release mocha v6, we may run tests with node v4.
I guess we shouldn't merge this commit to 5.x.x branch while master branch drops supporting node v4.

@outsideris outsideris mentioned this pull request May 2, 2018
Closed
@plroebuck
Copy link
Contributor

BTW, why was "appveyor.yml" left out of this PR?

@outsideris
Copy link
Contributor Author

No reason. I just missed it. updated!

plroebuck
plroebuck suggested changes May 4, 2018
View reviewed changes
Copy link
Contributor

@plroebuck plroebuck left a comment

Choose a reason for hiding this comment

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

For what it's worth, I think we should make our Node requirements against what we test against. As such, don't specify the entire semver - we never test against 6.0.0; we test against the latest 6.x release. So the "package.json" engines field should reflect that. And "README.md" should maybe just reference Node-6 (latest version).

@outsideris
Copy link
Contributor Author

Agreed.
To clear, we should support 6.x LTS boron only or entire 6.x versions, including 6.x before LTS. I'm also not sure that people think both, 6.x LTS and 6.x not LTS, are different.
There are no breaking changes in 6.x LTS releases. It's Node.js core team's responsibility. Therefore, we can test latest 6.x version while we can say we support 6.x LTS.

@boneskull boneskull force-pushed the master branch 2 times, most recently from 4547268 to 7613521 Compare May 4, 2018 16:47
@outsideris
drop node.js v4 support
549ff78 
Signed-off-by: Outsider <[email protected]>
@outsideris
Copy link
Contributor Author

I updated v6.x on docs.

@goteamtim goteamtim mentioned this pull request Jun 6, 2018
Merged
@boneskull boneskull merged commit cc8304d into mochajs:master Aug 10, 2018
@plroebuck plroebuck mentioned this pull request Nov 25, 2018
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@plroebuck plroebuck plroebuck requested changes

@boneskull boneskull boneskull approved these changes

Assignees
No one assigned
Labels
area: node.js command-line-or-Node.js-specific semver-major implementation requires increase of "major" version number; "breaking changes"
Projects
None yet
Milestone
v6.0.0
Development

Successfully merging this pull request may close these issues.

Drop Node.js 4.x support
4 participants
@outsideris @coveralls @plroebuck @boneskull