Fully disable task with --no-release-draft

[sonicdoe]

When running with --no-release-draft, fully disable the task instead of just skipping it, thereby completely hiding it from the task list. Similar options behave the same way.

For example, the “Cleanup” task is hidden when running with --no-cleanup:

$ np major

Publish a new version of foo (current: 1.0.0)


  ✔ Git
  ✔ Cleanup
  ✔ Installing dependencies using npm
  ✔ Running tests using npm
  ✔ Bumping version using npm
  ✔ Pushing tags
  ✔ Creating release draft on GitHub

 foo 2.0.0 published 🎉
$ np major --no-cleanup

Publish a new version of foo (current: 2.0.0)


  ✔ Git
  ✔ Running tests using npm
  ✔ Bumping version using npm
  ✔ Pushing tags
  ✔ Creating release draft on GitHub

 foo 3.0.0 published 🎉

However, the “Creating release draft on GitHub” task is listed as being skipped when running with --no-release-draft:

$ np major --no-release-draft

Publish a new version of foo (current: 3.0.0)


  ✔ Git
  ✔ Cleanup
  ✔ Installing dependencies using npm
  ✔ Running tests using npm
  ✔ Bumping version using npm
  ✔ Pushing tags
  ↓ Creating release draft on GitHub [skipped]

 foo 4.0.0 published 🎉

[itaisteinherz]

I'm a bit conflicted on this.
@sindresorhus I'd love to hear your thoughts on this - where should we use skip / enabled / a direct if (...) {tasks.add(...)}?

[sonicdoe]

To be clear, I’d be okay with both approaches and mostly opened this pull request to start a discussion around this behavior.

I’ve leaned towards completely hiding the task because, if one explicitly disables the task using --no-release-draft, they’re probably aware of what they’re doing (perhaps they don’t use GitHub releases at all) and don’t need to be reminded that the task is being skipped on every single run.

Edit: Of course, this still leaves the question whether to use enabled or an if statement open.

[sindresorhus]

I think it makes sense to completely hide it if the user explicitly passes a flag to do so. And skip it if it cannot run for some other reason, for example, if the repo is not GitHub. We should probably audit the codebase to ensure we do this consistently.

[sindresorhus]

Is there any difference between if () {} and enabled functionality-wise?

[sonicdoe]

And skip it if it cannot run for some other reason, for example, if the repo is not GitHub.

After giving it some thought, I wonder if this is actually what users would want. If I regularly use np to publish new versions of a package I host on GitLab, do I want to be reminded every time that my repository isn’t hosted on GitHub?

Is there any difference between if () {} and enabled functionality-wise?

Looking at listr’s source code, I don’t think there is. task.isEnabled() is called quite early.

[sindresorhus]

If I regularly use np to publish new versions of a package I host on GitLab, do I want to be reminded every time that my repository isn’t hosted on GitHub?

Good point. I guess there's no universal rule for this. We have to evaluate on a case-by-case basis. Can you do a best effort and we can discuss?

Looking at listr’s source code, I don’t think there is. task.isEnabled() is called quite early.

I guess we can just use enabled then, as long as it doesn't make the code more complex than using if. For example, if we need both if and else, it's probably better with an if/else statement.

[sindresorhus]

Bump :)

[sonicdoe]

We have to evaluate on a case-by-case basis. Can you do a best effort and we can discuss?

Most flags already follow the convention of completely hiding the task when explicitly specified (for example, --no-cleanup or --no-tests but also --yolo). Skipping the task is done in the following cases:

• “Cleanup”, if there’s a lock file (source)
• “Creating release draft on GitHub”, if the releaseDraft flag is falsy (source)
• “Ping npm registry”, if the package is private or hosted at an external registry (source)
• “Verify user is authenticated”, if NODE_ENV is test or the package is private (source)

I’d suggest changing all of them to enabled (or an if statement) because there’s arguably not much value in providing this information. For example, do I need to know node_modules hasn’t been deleted because I’m using a lock file? Do I need to know the npm registry hasn’t been pinged because my package isn’t meant to be published?

[sindresorhus]

Good points. I agree. Let's do that.

[sonicdoe]

As the goal shifted quite a bit, I’ve opened a new pull request to standardize this behavior of skipping and enabling tasks.