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

add test-node.js.yml to GitHub workflows #64

Merged
merged 1 commit into from Jun 28, 2020

Conversation

brodybits
Copy link
Member

from template on GitHub UI

with the following changes:

  • update filename
  • update name field
  • replace npm ci with npm install
  • remove npm run build step

This should get CI testing working again both on master branch and in PRs

This is in followup to PR #59

/cc @karfau

from template on GitHub UI

with the following changes:

- update filename
- update name field
- replace `npm ci` with `npm install`
- remove `npm run build` step
@brodybits brodybits added the bug Something isn't working label Jun 28, 2020
@brodybits brodybits self-assigned this Jun 28, 2020
@brodybits
Copy link
Member Author

I am taking the liberty to merge immediately since I think this is needed for contributions coming in the near future.

@brodybits brodybits merged commit 5883af5 into master Jun 28, 2020
@brodybits brodybits deleted the brodybits-add-test-action branch June 28, 2020 18:30
@karfau
Copy link
Member

karfau commented Jun 28, 2020

Just one question: why did you choose npm install over npm ci?
(My understanding would be that we should always check what is in the package-lock.json and npm install can change that file if there is something newer in the specified ranges.)

Reference: https://blog.npmjs.org/post/171556855892/introducing-npm-ci-for-faster-more-reliable

@brodybits
Copy link
Member Author

why did you choose npm install over npm ci?

That was an unfortunate oversight on my part. Some of my own projects are using yarn.lock instead, and some others do not commit any lock file; npm ci would not work in either of these cases. Please feel free to raise a new PR to fix it, if you like.

This was referenced Mar 9, 2021
This was referenced Mar 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants