search

ilearnio / module-alias

Register aliases of directories and custom module paths in Node
MIT License
1.76k stars 69 forks source link

Added GitHub Action workflows to run the tests on various versions of Node.js #126

Closed jamesmortensen closed 2 years ago

jamesmortensen commented 2 years ago

I created two workflow files, one uses the node-setup action to run through all of the versions of Node in the matrix list and run the tests. All tests passed except on v5.x and v0.10.0 due to npm incompatibilities.

nvm has a feature that lets you update npm to the latest known working version with a specific node version. So I created another workflow to use nvm to install various node versions instead of the node-setup action. Specifically for v5.x and v0.10.0, I ran nvm install-latest-npm to update npm to a working version.

This results in tests passing for v5.x, but there are certificate errors when attempting to npm install using Node v0.10.0. You can see the results here: https://github.com/jamesmortensen/module-alias/runs/5581223733?check_suite_focus=true

Not sure how to proceed from here, so I thought I'd share what I'd done so far.

Kehrlann commented 2 years ago

Hey @jamesmortensen 👋

Thanks a lot for contributing, and taking the time to dig into this. My thoughts so far:

  1. I think we're fine with whatever Github gives us as a node version, no need to go the extra mile with the nvm-managed versions. Let's keep the nodejs workflow.
    1. It's equivalent to what we had with Travis CI
    2. And it's much better than what we have now ~ nothing 😅
  2. Let's get this merged asap:
    1. Remove 5.0 and 0.10
    2. Keep the LTS: 6 (it passes locally, I tried), 8, 10, 12, 14, 16
    3. Add 17 (why not?)
  3. Rename the worfklow test
  4. Remove .travis.yml as part of your PR

I think I'm inclined towards cutting a 3.0.0 release, dropping support for older node versions. I don't have the bandwidth to dig into testing issues on node versions that were End-Of-Life'd in 2016. I'll sleep on it.

Also, if you're interested, feel free to open another PR with a lint workflow, that runs lint with the latest stable node version.

jamesmortensen commented 2 years ago

Ok, @Kehrlann I'll make the changes this next week when I get more time to look at this. For what it's worth, it seems like a hassle to support 0.10.0 and 5.x, so... great call!

James

jamesmortensen commented 2 years ago

@Kehrlann I made the changes.

Looking forward to the 3.0.0 release!

jamesmortensen commented 2 years ago

Hi @Kehrlann I just wanted to bump this thread. Are we ready to merge this pull request? Please let me know if there is anything else we need to change.

jamesmortensen commented 2 years ago

Hi @Kehrlann or @ilearnio is it possible for me to merge this without you? If tests are passing, is there anything else we need to do?

Kehrlann commented 2 years ago

Hey @jamesmortensen, I'm really sorry. For some reason, I did not get notified about new replies in this thread.

Looks good to me, I'll squash and merge it. Thanks for contributing!

Kehrlann commented 2 years ago

Closes #115

jamesmortensen commented 2 years ago

No worries! Glad to see this merged. Hope it helps clear the other PR's now that tests can be more easily run.