[MAJOR] Convert to ES Modules #95
Conversation
Pull Request Test Coverage Report for Build 1137078901
💛 - Coveralls |
Co-authored-by: Bradley Farias <bradley.meck@gmail.com>
.github/workflows/ci.yml
Outdated
| @@ -16,7 +16,7 @@ jobs: | |||
|
|
|||
| strategy: | |||
| matrix: | |||
| node-version: [10.x, 12.x, 14.x] | |||
| node-version: [12.x, 14.x, 16.x] | |||
There was a problem hiding this comment.
Does it still need to support node@12? You're updating ESLint to ecma version 2020, but I don't believe node@12 supports most of that. A new major version would be a great time to drop support for an older node version. That said EOL for node@12 isn't until 2022-04-30
There was a problem hiding this comment.
Nothing says that I have to support ALL Node.js versions -- might as well just cull down to 14 and 16.
test/unit/config-processor.test.js
Outdated
| import sinon from 'sinon'; | ||
|
|
||
| assume.use(assumeSinon); | ||
|
|
||
| import processConfig, { applyExcludeList, applyIncludeList } from '../../config-processor.js'; |
There was a problem hiding this comment.
IMHO stylistically, imports belong above code.
Also IMHO move the assume.use(assumeSinon) into a global test setup so you don't have to do it everywhere or assume some file executed before another (i.e. this is leaky)
There was a problem hiding this comment.
I like to do this only in the test files that use sinon as the typings for assume-sinon extend the assume types in VS Code, so the import actually does something.
test/unit/plugins/welcome.test.js
Outdated
| assume.use(assumeSinon); | ||
|
|
||
| import WelcomePlugin from '../../../plugins/welcome/index.js'; | ||
| import Commenter from '../../../commenter.js'; | ||
|
|
||
| const sandbox = sinon.createSandbox(); |
There was a problem hiding this comment.
Do you still need a sinon sandbox? 99% of the time you just want to use the default sandbox of sinon itself
This PR converts Pullie to native ECMAScript Modules (from CommonJS).