Address review comments

* Add nice error when `$EDITOR` is not set properly, but `launchEditor:
  true` is specified.
* Refactor README documentation of options to make it clearer what is
  allowed
* Force `ava` to run serially (without this the tests run in parallel
  and mutate `process.env` causing failures dependent on timing).
* Update `_execLernaChangelog` signature to be less nuts

README.md

@@ -64,11 +64,24 @@ For example, given the following configuration (in `package.json`):
 
 The two options that `release-it-lerna-changelog` is aware of are:
 
-* `infile` -- This represents the filename to put the changelog information into.
-* `launchEditor` -- When set to `true`, `release-it-lerna-changelog` will
-  invoke the `$EDITOR` from your environment passing it the path to a temp file
-  that you can edit to customize the exact changelog contents. When set to a string,
-  that specific editor will be executed (as opposed to leveraging `$EDITOR`).
+### `infile`
+
+`infile` represents the file to prepend the generated changelog into.
+
+### `launchEditor`
+
+When specified, `release-it-lerna-changelog` will generate the changelog
+then launch the configured editor with a temporary file. This allows the person
+doing the release to customize the changelog before continuing.
+
+There are a few valid values for `launchEditor`:
+
+* `false` - Disables the feature.
+* `true` - The `process.env.EDITOR` value will be used as the command, and the
+  temporary file for editing is added as a argument (i.e. `$EDITOR /some/tmp/file`).
+* any string - This string will be used as if it were a command. In order to
+  interpolate the temporary file path in the string, you can use `${file}` in
+  your configuration.
 
 Each release will run `lerna-changelog` and prepend the results into `CHANGELOG.md`.
 

index.js

@@ -35,7 +35,7 @@ module.exports = class LernaChangelogGeneratorPlugin extends Plugin {
     return firstCommit;
   }
 
-  async _execLernaChangelog(nextVersion, from) {
+  async _execLernaChangelog(from, nextVersion) {
     let changelog = await this.exec(
       `${this.lernaPath} --next-version=${nextVersion} --from=${from}`,
       {
@@ -55,7 +55,7 @@ module.exports = class LernaChangelogGeneratorPlugin extends Plugin {
       from = await this.getFirstCommit();
     }
 
-    let changelog = await this._execLernaChangelog(nextVersion, from);
+    let changelog = await this._execLernaChangelog(from, nextVersion);
 
     let finalChangelog = await this.reviewChangelog(changelog);
 
@@ -66,8 +66,18 @@ module.exports = class LernaChangelogGeneratorPlugin extends Plugin {
     let editorCommand;
 
     if (typeof this.options.launchEditor === 'boolean') {
+      let EDITOR = process.env.EDITOR;
+      if (!EDITOR) {
+        let error = new Error(
+          `release-it-lerna-changelog configured to use $EDITOR but no $EDITOR was found`
+        );
+        this.log.error(error.message);
+
+        throw error;
+      }
+
       // `${file}` is actually interpolated by `this.exec`
-      editorCommand = process.env.EDITOR + ' ${file}';
+      editorCommand = EDITOR + ' ${file}';
     } else {
       editorCommand = this.options.launchEditor;
     }

package.json

@@ -21,6 +21,9 @@
     "lint:js": "eslint .",
     "test": "ava"
   },
+  "ava": {
+    "serial": true
+  },
   "dependencies": {
     "lerna-changelog": "^1.0.1",
     "release-it": "^13.0.2",

test.js

@@ -3,14 +3,18 @@ const tmp = require('tmp');
 const test = require('ava');
 const { factory, runTasks } = require('release-it/test/util');
 const Plugin = require('./index');
-const EDITOR = process.env.EDITOR;
+const EDITOR = process.env.EDITOR || null;
 
 tmp.setGracefulCleanup();
 
 const namespace = 'release-it-lerna-changelog';
 
 function resetEDITOR() {
-  process.env.EDITOR = EDITOR;
+  if (EDITOR === null) {
+    delete process.env.EDITOR;
+  } else {
+    process.env.EDITOR = EDITOR;
+  }
 }
 
 class TestPlugin extends Plugin {
@@ -115,7 +119,7 @@ test('uses launchEditor command', async t => {
 test('detects default editor if launchEditor is `true`', async t => {
   let infile = tmp.fileSync().name;
 
-  let plugin = buildPlugin({ infile, launchEditor: true }, TestPlugin);
+  let plugin = buildPlugin({ infile, launchEditor: true });
 
   try {
     process.env.EDITOR = 'foo-editor -w';
@@ -131,6 +135,30 @@ test('detects default editor if launchEditor is `true`', async t => {
   }
 });
 
+test('throws if launchEditor is `true` and no $EDITOR present', async t => {
+  let infile = tmp.fileSync().name;
+
+  let plugin = buildPlugin({ infile, launchEditor: true });
+
+  try {
+    delete process.env.EDITOR;
+
+    await runTasks(plugin);
+  } catch (error) {
+    t.deepEqual(plugin.commands, [
+      [`git show-ref --tags --quiet --verify -- "refs/tags/1.0.0"`, { write: false }],
+      [`${plugin.lernaPath} --next-version=1.0.1 --from=1.0.0`, { write: false }],
+    ]);
+
+    t.is(
+      error.message,
+      `release-it-lerna-changelog configured to use $EDITOR but no $EDITOR was found`
+    );
+  } finally {
+    resetEDITOR();
+  }
+});
+
 test('launches configured editor, updates infile, and propogates changes to context', async t => {
   class TestPlugin extends Plugin {
     constructor() {