Move scratch-parser to GitHub Actions#67
Conversation
chore: bump node v chore: include the package-lock chore: split the lint and test commands
ca19152 to
213f4fe
Compare
cwillisf
left a comment
cwillisf
left a comment
There was a problem hiding this comment.
Looks great overall! One question / potential change...
| "test:coverage": "tap test/{unit,integration}/*.js --coverage --coverage-report=lcov", | ||
| "test:benchmark": "node test/benchmark/performance.js", | ||
| "test": "npm run test:lint && npm run test:unit && npm run test:integration", | ||
| "test": "npm run test:unit && npm run test:integration", |
There was a problem hiding this comment.
Why separate test:lint from test? Seems like undoing this change and removing the separate npm run test:lint step in the workflow would work...?
There was a problem hiding this comment.
I always think of linting and testing as two different things, so my preference is to separate those two scripts. I like the separation in the workflow so that I can tell what event had issues. I would even go as far as saying unit and integration tests are two different things. :)
There was a problem hiding this comment.
Heh - fair enough. I definitely agree that including linting in npm test is annoying when you're working on the tests!
|
🎉 This PR is included in version 5.2.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
2 participants
Resolves
https://scratchfoundation.atlassian.net/browse/ENG-41