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

Supporting Node12 and above #34

Closed
HappyZombies opened this issue Oct 12, 2021 · 11 comments · Fixed by #35 or #41
Closed

Supporting Node12 and above #34

HappyZombies opened this issue Oct 12, 2021 · 11 comments · Fixed by #35 or #41
Labels
completed 😀 Work that has been completed discussion 🗨️ Discussion about a particular topic.

Comments

@HappyZombies
Copy link
Member

Apparently, we (master) support Node 4 and above. We should, however, consider following the Node LTS and basically, support Node 12 and above.

The simplest change would be updating the package.json to update the engines to node >= 12.0.

The question is now What and where else would we need to make changes to show we support only Node12 and above? (obviously CI/CD testing, but that convo is for another issue).

Some things that we are merging to development only support Node 12 and above (example, mocha). So would this change be backwards breaking?

A fun little tidbit is that in the previous repository we forked, the travis.yml file actually was failing on Node 4 - Node 10...so maybe we already support Node 12 but just need to update the docs and package.json? https://travis-ci.org/github/oauthjs/node-oauth2-server

@HappyZombies HappyZombies added the discussion 🗨️ Discussion about a particular topic. label Oct 12, 2021
@jankapunkt
Copy link
Member

jankapunkt commented Oct 12, 2021

We already support Node 12 in the CI and I am running all tests on Node 14 locally so we are fine here.

Also we can be fine by removing bluebird without any replacement and just use native Promises, which will be already breaking for very old Node versions. Related: #19

Edit: also related: #23 because we would need native ES6 functionality which is not fully supported in Node 4 at least

@HappyZombies
Copy link
Member Author

Understood, I guess this issue was more for dropping support for versions below Node 12 really.

@jwerre
Copy link
Contributor

jwerre commented Oct 12, 2021

@jankapunkt I was looking at the workflow you set up and I see that it's only testing on node 12. I wonder if it would be better to test in multiple versions? What do you think? Also, see previous comment.

Originally posted by @jwerre in #5 (comment)

@jwerre
Copy link
Contributor

jwerre commented Oct 12, 2021

I think we would be fine supporting Node 10. Is there anything here that you guys feel like we need?

@jankapunkt
Copy link
Member

I can update the ci to support 10,12,14 and 16

@jwerre
Copy link
Contributor

jwerre commented Oct 12, 2021

@jankapunkt... Also, we should be good to remove the travis.yaml correct?

@HappyZombies
Copy link
Member Author

I think removing the Travis file is fine. We are using GitHub CI now.

@jankapunkt
Copy link
Member

PR is open :-) #35

@jwerre
Copy link
Contributor

jwerre commented Oct 13, 2021

We should change this to Node 12:

@jankapunkt
Copy link
Member

The travis.yml is removed. Should I set the node version up to >=12 @jwerre ?

@HappyZombies
Copy link
Member Author

@jankapunkt I totally overlooked this, we can make another PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
completed 😀 Work that has been completed discussion 🗨️ Discussion about a particular topic.
Projects
None yet
3 participants