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.

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.

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 ❤️)

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.

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

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 ❤️)