-
Notifications
You must be signed in to change notification settings - Fork 6
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
feat: adding initial test structure #4
Conversation
This initial structure includes github action and jest using |
updated the PR adding husky as the second part of adding code consistency from ref arch. |
PR updated again with husky flow working |
|
||
strategy: | ||
matrix: | ||
node-version: [10.x, 12.x, 14.x] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@helio-frota some of npcheck's plugins use the Optional Chaining operator (https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Optional_chaining) which is only available in node v14+
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks for the information. I'll change that 👍
@@ -17,8 +17,10 @@ | |||
"npcheck": "index.js" | |||
}, | |||
"scripts": { | |||
"prepare": "husky install", | |||
"pretest": "eslint --ignore-path .gitignore", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a minor detail (also personal preference I admit) but I think we should stick with the lint
naming instead of pretest
(e.x npm run lint)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is part of the husky flow, the commit will call the test which calls the lint via pretest.
https://github.com/nodeshift/nodejs-reference-architecture/blob/main/docs/development/code-consistency.md#guidance
I keep the "lint:fix" in case dev preference.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
We are using --no-colors to make the test pass by avoiding colors to make interference related to PASS FAIL output. Since we transferred the coverage bits to coveralls action seems like we don't need that script anymore. Also the script is broken and seems unused since April #4
No description provided.