Adding yaml as a dep in prepare-node-test-matrix-action
#28
Conversation
| @@ -5,7 +5,7 @@ | |||
| "description": "Github Actions tooling for testing Node.js packages", | |||
| "scripts": { | |||
| "lint": "[ \"$NODE_LTS_LATEST\" != \"\" ] && [ \"$MATRIX_NODE_VERSION\" != \"$NODE_LTS_LATEST\" ] && echo 'Skipping linting' || npx -- eslint .github", | |||
| "test": "for PRIVATE_ACTION in .github/actions/*/; do cd ${INIT_CWD}/${PRIVATE_ACTION} && npm test; done && cd ${INIT_CWD} && npm run lint" | |||
| "test": "for PRIVATE_ACTION in .github/actions/*/; do cd ${INIT_CWD}/${PRIVATE_ACTION} && npm test || exit 1; done && cd ${INIT_CWD} && npm run lint" | |||
There was a problem hiding this comment.
why is this needed? npm test will exit nonzero, as will cd, if either fails, which should fail the overarching for?
There was a problem hiding this comment.
It does not fail the overarching for 🤷♂️
Possibly due the semicolon before done? Not sure if doing && done would work 🤔
There was a problem hiding this comment.
ah, no, you're right, i guess that's how it works.
any reason not to use npm workspaces, or lerna?
There was a problem hiding this comment.
Mostly because I think going forward I'll want to extract at least the matrix generator into a separate repo.
There was a problem hiding this comment.
That seems like a stronger argument for using a workspaces tool, because then each subpackage has its own package.json. Your call tho.
There was a problem hiding this comment.
They do have their own package.json even now.
But good point - I might revisit after #6 is implemented (need npm 8 on node 12 to achieve that).
Thanks for the review.
I'll need this in #29 - Github Actions needs all dependencies in the repo, so the
node_moduleschanges are cluttering the actual code changes.Also fixing the test script to properly exit.