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

Use Node 16/18 and NPM #42

Merged
merged 7 commits into from
Apr 24, 2023
Merged

Use Node 16/18 and NPM #42

merged 7 commits into from
Apr 24, 2023

Conversation

cmoesel
Copy link
Member

@cmoesel cmoesel commented Apr 24, 2023

This PR removes yarn in favor of npm. The yarn.lock is replaced by package-lock.json. The GitHub actions and README have also been updated. In addition, a new test:plus script has been added for running tests, lint, and prettier in one shot.

To test, pull the branch and delete node_modules. Make sure you're running Node 18, then run npm install followed by npm test:plus. Everything should work well. Note that Node 14 and 16 should also work (and are tested in CI), but we avoid them in development since they use a different package-lock format.

@cmoesel cmoesel requested a review from jafeltra April 24, 2023 18:04
Copy link

@jafeltra jafeltra left a comment

Choose a reason for hiding this comment

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

This looks good to me. I left one comment that you can take or leave. Besides that, I didn't see anything else in the code.

I'm not sure how strict you tend to be in PR descriptions in this repo, but just so you know, I think you're referring to the wrong npm script in the description:

Make sure you're running Node 18, then run npm install followed by npm check

should probably be

Make sure you're running Node 18, then run npm install followed by npm run test:plus

cmoesel added 2 commits April 24, 2023 17:24
Using npm ci chokes w/ node 14 -- so use npm install.

Also update checkout and setup-node actions to use v3.
@cmoesel cmoesel merged commit 2828dac into master Apr 24, 2023
@cmoesel cmoesel deleted the use-npm branch April 24, 2023 21:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants