Skip to content
This repository has been archived by the owner on Jan 24, 2022. It is now read-only.

Option --no-interactive should not mean false #1544

Open
kigawas opened this issue May 25, 2020 · 1 comment
Open

Option --no-interactive should not mean false #1544

kigawas opened this issue May 25, 2020 · 1 comment
Labels
topic:cli Related to OpenZeppelin CLI

Comments

@kigawas
Copy link
Contributor

kigawas commented May 25, 2020

It is assumed in a pile of places that interactive: false is equal to false, but it's not always correct.

I'll post some codes here.

// cli/src/commands/push.ts
async function runActionIfNeeded(contracts: string[], network: string, options: any): Promise<void> {
  if (!options.interactive) return; // why return?
  await action({ ...options, dontExitProcess: true, skipTelemetry: true, contracts });
}

async function promptForDeployDependencies(
  deployDependencies: boolean,
  network: string,
  interactive: boolean,
): Promise<{ deployDependencies: boolean | undefined }> {
  if (await ZWeb3.isGanacheNode()) return { deployDependencies: true };
  if (Dependency.hasDependenciesForDeploy(network)) {
    const opts = { deployDependencies };
    const props = getCommandProps(network);
    return promptIfNeeded({ opts, props }, interactive);
  }
  return { deployDependencies: undefined }; // why not true? even it's false, why undefined?
}

The code above causes the bug in #1538 , when --no-interactive enabled, it'll skip pushing dependencies.

@kigawas kigawas changed the title Opiton --no-interactive does not necessarily mean false Option --no-interactive does not necessarily mean false May 25, 2020
@frangio frangio added the topic:cli Related to OpenZeppelin CLI label May 26, 2020
@frangio
Copy link
Contributor

frangio commented May 26, 2020

Thanks for reporting @kigawas. This is definitely the case. Initially the reason was that under the interactivity umbrella we introduced many breaking changes to the behavior of the CLI, and --no-interactive was a way to opt out of the whole package. At this point this is only a historical artifact and it has unintuitive consequences that are harming usability.

@frangio frangio changed the title Option --no-interactive does not necessarily mean false Option --no-interactive should not mean false May 26, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
topic:cli Related to OpenZeppelin CLI
Projects
None yet
Development

No branches or pull requests

2 participants