chore(deps): drop mz dependency
#3215
Conversation
| @@ -1,6 +1,7 @@ | |||
| import path from 'path'; | |||
| import nodeFs from 'fs'; | |||
| import fs from 'fs/promises'; | |||
There was a problem hiding this comment.
Sometimes both APIs are used. For consistency, I let fs stay the promised API and used nodeFs for the other APIs. This name was actually used elsewhere too.
| @@ -1,6 +1,7 @@ | |||
| import path from 'path'; | |||
| import fs from 'fs/promises'; | |||
| import { writeFileSync } from 'fs'; | |||
There was a problem hiding this comment.
In some cases I used the single export instead. This pattern was also already used in parts of the repo. However this is not always possible due to stubbing.
There was a problem hiding this comment.
Makes sense (and thanks a lot for mentioning the motivations behind these choices explicitly in this comment, that really helped to speed up the review and sign off ❤️)
|
Due to the number of touched files, it would be good to merge this sooner rather than later |
| it('returns git commit information in development', function () { | ||
| return fs.exists(path.join(projectRoot, '.git')).then(async (exists) => { | ||
| if (!exists) { | ||
| return fs.access(path.join(projectRoot, '.git')).then( | ||
| async () => { | ||
| const commit = `${git.branch(projectRoot)}-${git.long(projectRoot)}`; | ||
| const testBuildEnv = { globalEnv: 'development' }; | ||
| assert.equal( | ||
| await defaultVersionGetter(projectRoot, testBuildEnv), | ||
| commit, | ||
| ); | ||
| }, | ||
| () => { | ||
| this.skip(); | ||
| } | ||
| const commit = `${git.branch(projectRoot)}-${git.long(projectRoot)}`; | ||
| const testBuildEnv = { globalEnv: 'development' }; | ||
| assert.equal( | ||
| await defaultVersionGetter(projectRoot, testBuildEnv), | ||
| commit, | ||
| ); | ||
| }); | ||
| }, | ||
| ); | ||
| }); |
There was a problem hiding this comment.
Nit: not really mandatory, but given that we need to touch this test case a bit then we may as well rewrite into an async function, and make it slightly more readable:
it('returns git commit information in development', async function () {
try {
await fs.access(path.join(projectRoot, '.git'));
} catch {
this.skip();
return;
}
const commit = `${git.branch(projectRoot)}-${git.long(projectRoot)}`;
const testBuildEnv = { globalEnv: 'development' };
assert.equal(
await defaultVersionGetter(projectRoot, testBuildEnv),
commit,
);
});but I'd also like to merge this sooner rather than later, and in practice the test would be basically the same, and so we can eventually do that later after this is merged.
There was a problem hiding this comment.
Definitely, in my PRs here I just limited my changes to the bare minimum to avoid unnecessary bikeshedding.
I think both this repo and addons-linter would benefit from a strict lint config and a whole bunch of rules (like eslint-plugin-unicorn) with autofixes.
Your suggestion is https://github.com/eslint-community/eslint-plugin-promise/blob/main/docs/rules/prefer-await-to-then.md
| @@ -1,6 +1,7 @@ | |||
| import path from 'path'; | |||
| import fs from 'fs/promises'; | |||
| import { writeFileSync } from 'fs'; | |||
There was a problem hiding this comment.
Makes sense (and thanks a lot for mentioning the motivations behind these choices explicitly in this comment, that really helped to speed up the review and sign off ❤️)
https://github.com/normalize/mz
fs/promisesis node v14+, no need formzanymore.Refer to the 2 existing review comments if you have questions.