[CLI] Leo CLI is now on StructOpt and Anyhow (reopened) #632

damirka · 2021-02-08T13:09:15Z

This PR is a reopened #618 to support CI secrets in login workflow

Features

introduces new Command and Route traits for Leo commands and Aleo PM API
most of the CLI code replace with higher-level abstraction - StructOpt
anyhow used for error handling, no more custom error classes
improves API - now every status code has its business logic
adds global flags (e.g. --quiet to suppress output)
error messages improved for convenience and better user experience

Closes

Test Plan

Includes integration tests for leo binary and patches to GitHub Actions workflows.

Features: - introduces new Command and Route traits for Leo commands and Aleo PM API - most of the CLI code replace with higher-level abstraction - StructOpt - anyhow used for error handling, no more custom error classes - improves API - now every status code has its business logic - adds global flags (e.g. --quiet to suppress output) - error messages improved for convenience and better user experience Closes: - #604 - #599 - #584 - #277 - #376

codecov · 2021-02-08T13:41:34Z

Codecov Report

Merging #632 (cb050aa) into master (1898cc6) will decrease coverage by 0.71%.
The diff coverage is 80.57%.

@@            Coverage Diff             @@
##           master     #632      +/-   ##
==========================================
- Coverage   75.69%   74.97%   -0.72%     
==========================================
  Files         518      549      +31     
  Lines       16135    17258    +1123     
==========================================
+ Hits        12214    12940     +726     
- Misses       3921     4318     +397

Impacted Files	Coverage Δ
leo/lib.rs	`100.00% <ø> (ø)`
leo/main.rs	`100.00% <ø> (ø)`
leo/synthesizer/serialized_circuit.rs	`59.74% <ø> (ø)`
leo/synthesizer/serialized_field.rs	`0.00% <ø> (ø)`
leo/synthesizer/serialized_index.rs	`0.00% <ø> (ø)`
leo/commands/package/login.rs	`52.94% <52.94%> (ø)`
leo/api.rs	`62.16% <62.16%> (ø)`
leo/commands/package/logout.rs	`56.25% <66.66%> (ø)`
leo/commands/update.rs	`77.77% <75.00%> (ø)`
leo/commands/setup.rs	`52.94% <78.57%> (ø)`
... and 95 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1898cc6...2c5c2eb. Read the comment docs.

leo/commands/prove.rs

leo/commands/setup.rs

howardwu · 2021-02-09T04:26:16Z

leo/updater.rs

@@ -27,7 +28,7 @@ impl Updater {
    const LEO_REPO_OWNER: &'static str = "AleoHQ";

    /// Show all available releases for `leo`.
-    pub fn show_available_releases() -> Result<(), UpdaterError> {
+    pub fn show_available_releases() -> Result<(), Error> {


Does this change to anyhow::Error produce the same UpdaterError message in the terminal as before (if any)?

Here are the outputs (when no network available):

Current version:

Updating ReqwestError: error sending request for url (https://api.github.com/repos/AleoHQ/leo/releases/latest): error trying to connect: dns error: failed to lookup address information: nodename nor servname provided, or not known Updating Could not update Leo to the latest version Updating self_update: ReqwestError: error sending request for url (https://api.github.com/repos/AleoHQ/leo/releases/latest): error trying to connect: dns error: failed to lookup address information: nodename nor servname provided, or not known

This update:

Updating Could not update Leo to the latest version Updating ReqwestError: error sending request for url (https://api.github.com/repos/AleoHQ/leo/releases/latest): error trying to connect: dns error: failed to lookup address information: nodename nor servname provided, or not known

howardwu · 2021-02-09T04:27:29Z

leo/main.rs

+    #[structopt(about = "Init new Leo project command in current directory")]
+    Init {
+        #[structopt(flatten)]
+        cmd: Init,


nit: please rename cmd to command

question: if we're telling structopt to simply flatten the command, is there a reason we need to initialize the inner cmd?

Done.

We need to, yes. Flattening allows using external structs as subcommands + see how this code is called within main function. It also gives an option for grouping if we'll want to.

howardwu · 2021-02-09T04:30:13Z

leo/commands/setup.rs

    source::{MAIN_FILENAME, SOURCE_DIRECTORY_NAME},
 };

+use rand::thread_rng;


nit: please organize the imports here into two sections - leo specific crates & external crates

Here and below: used rustfmt rule for grouping imports.

howardwu · 2021-02-09T04:30:21Z

leo/commands/run.rs

-use crate::{cli::*, cli_types::*, commands::ProveCommand, errors::CLIError};
+use crate::{commands::Command, context::Context};
+
+use anyhow::Result;


nit: please organize the imports here into two sections - leo specific crates & external crates

howardwu · 2021-02-09T04:30:27Z

leo/commands/prove.rs

-use leo_package::{outputs::ProofFile, root::Manifest};
+use super::setup::Setup;
+use crate::{commands::Command, context::Context};
+use anyhow::Result;


nit: please organize the imports here into two sections - leo specific crates & external crates

howardwu · 2021-02-09T04:30:51Z

leo/commands/package/publish.rs

 };

-use clap::ArgMatches;
+use anyhow::{anyhow, Result};


nit: please organize the imports here into two sections - leo specific crates & external crates

howardwu · 2021-02-09T04:30:57Z

leo/commands/package/logout.rs

-
-use crate::{cli::CLI, cli_types::*, config::remove_token, errors::CLIError};
+use crate::config::remove_token;
+use anyhow::Result;


nit: please organize the imports here into two sections - leo specific crates & external crates

howardwu · 2021-02-09T04:31:28Z

leo/commands/package/add.rs

+use leo_package::imports::{ImportsDirectory, IMPORTS_DIRECTORY_NAME};
+use tracing::Span;
+
+use std::{
+    fs::{create_dir_all, File},
+    io::{Read, Write},
+};
+
+use crate::{commands::Command, context::Context};
+
+use anyhow::{anyhow, Result};
+use structopt::StructOpt;
+
+use crate::api::Fetch;


nit: please organize the imports here into two sections - leo specific crates & external crates

howardwu · 2021-02-09T04:31:36Z

leo/commands/new.rs


-use clap::ArgMatches;
+use leo_package::LeoPackage;


nit: please organize the imports here into two sections - leo specific crates & external crates

howardwu · 2021-02-09T04:31:56Z

leo/commands/mod.rs

+pub mod watch;
+pub use watch::Watch;

 pub mod update;
-pub use self::update::*;
+pub use update::{Sub as UpdateAutomatic, Update};

-pub mod watch;
-pub use self::watch::*;
+// aleo pm related commands
+pub mod package;
+
+// not implemented
+pub mod deploy;
+pub use deploy::Deploy;
+
+pub mod lint;
+pub use lint::Lint;


nit: please sort alphabetically

howardwu · 2021-02-09T04:32:03Z

leo/commands/mod.rs

-pub mod add;
-pub use self::add::*;
+use crate::context::{get_context, Context};
+use anyhow::Result;


nit: please organize the imports here into two sections - leo specific crates & external crates

howardwu · 2021-02-09T04:32:11Z

leo/commands/lint.rs

-#[derive(Debug)]
-pub struct LintCommand;
+use crate::{commands::Command, context::Context};
+use anyhow::Result;


nit: please organize the imports here into two sections - leo specific crates & external crates

howardwu · 2021-02-09T04:32:16Z

leo/commands/init.rs

-    errors::{CLIError, InitError},
-};
+use crate::{commands::Command, context::Context};
+use anyhow::{anyhow, Result};


nit: please organize the imports here into two sections - leo specific crates & external crates

howardwu · 2021-02-09T04:32:19Z

leo/commands/deploy.rs

-#[derive(Debug)]
-pub struct DeployCommand;
+use crate::{commands::Command, context::Context};
+use anyhow::Result;


nit: please organize the imports here into two sections - leo specific crates & external crates

howardwu · 2021-02-09T04:32:24Z

leo/commands/clean.rs

-
-use clap::ArgMatches;
+use crate::{commands::Command, context::Context};
+use anyhow::Result;


nit: please organize the imports here into two sections - leo specific crates & external crates

howardwu · 2021-02-09T04:32:53Z

leo/api.rs

+use serde::Serialize;
+
+use anyhow::{anyhow, Error, Result};
+use reqwest::{
+    blocking::{Client, Response},
+    Method,
+    StatusCode,
+};


nit: please combine for rustfmt to sort

Don't understand this one. What do you mean by combining?

examples/pedersen-hash/src/main.leo

- fixes import ordering (with rule group_imports) - removes explicit 0group from pedersen hash example - updater is now fully-anyhow-ed :) - fixed doc comment in prove command - renamed cmd to command in main

- leo new/init now don't have --lib flag - imports sorted - command descriptions match old ones 1-to-1

leo/main.rs

damirka added feature A new feature. refactor Refactor or cleanup. ci labels Feb 8, 2021

damirka requested review from damons, Protryon, howardwu and collinc97 February 8, 2021 13:09

damirka self-assigned this Feb 8, 2021

damirka mentioned this pull request Feb 8, 2021

[CLI] StructOpt added, code reorganized #618

Closed

12 tasks

howardwu reviewed Feb 9, 2021

View reviewed changes

leo/commands/prove.rs Outdated Show resolved Hide resolved

howardwu reviewed Feb 9, 2021

View reviewed changes

leo/commands/setup.rs Outdated Show resolved Hide resolved

howardwu reviewed Feb 9, 2021

View reviewed changes

examples/pedersen-hash/src/main.leo Outdated Show resolved Hide resolved

damirka mentioned this pull request Feb 9, 2021

[Feature] Add group_imports rule to .rustfmt.toml #636

Closed

Follow up on comments to PR

deb830c

- fixes import ordering (with rule group_imports) - removes explicit 0group from pedersen hash example - updater is now fully-anyhow-ed :) - fixed doc comment in prove command - renamed cmd to command in main

damirka requested a review from howardwu February 9, 2021 16:58

damirka added 2 commits February 10, 2021 19:18

imports sorting, --lib flag removed

7867ab9

- leo new/init now don't have --lib flag - imports sorted - command descriptions match old ones 1-to-1

removed unused constants from add

cb050aa

collinc97 mentioned this pull request Feb 10, 2021

[Bug] leo login requires zv #592

Closed

collinc97 added the developer-experience label Feb 10, 2021

howardwu reviewed Feb 11, 2021

View reviewed changes

leo/main.rs Outdated Show resolved Hide resolved

Update leo/main.rs

2c5c2eb

howardwu approved these changes Feb 11, 2021

View reviewed changes

collinc97 approved these changes Feb 11, 2021

View reviewed changes

howardwu merged commit 59fa8fd into master Feb 11, 2021

damirka added the update-docs label Feb 22, 2021

damirka mentioned this pull request Feb 22, 2021

[Docs] Update developer docs - DP II #684

Closed

9 tasks

howardwu deleted the adds-structopt-to-leo branch February 25, 2021 06:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[CLI] Leo CLI is now on StructOpt and Anyhow (reopened) #632

[CLI] Leo CLI is now on StructOpt and Anyhow (reopened) #632

damirka commented Feb 8, 2021

codecov bot commented Feb 8, 2021 •

edited

Loading

howardwu Feb 9, 2021

damirka Feb 9, 2021 •

edited

Loading

howardwu Feb 9, 2021

damirka Feb 9, 2021 •

edited

Loading

howardwu Feb 9, 2021

damirka Feb 9, 2021 •

edited

Loading

howardwu Feb 9, 2021

howardwu Feb 9, 2021

howardwu Feb 9, 2021

howardwu Feb 9, 2021

howardwu Feb 9, 2021

howardwu Feb 9, 2021

howardwu Feb 9, 2021

howardwu Feb 9, 2021

howardwu Feb 9, 2021

howardwu Feb 9, 2021

howardwu Feb 9, 2021

howardwu Feb 9, 2021

howardwu Feb 9, 2021

damirka Feb 9, 2021 •

edited

Loading

[CLI] Leo CLI is now on StructOpt and Anyhow (reopened) #632

[CLI] Leo CLI is now on StructOpt and Anyhow (reopened) #632

Conversation

damirka commented Feb 8, 2021

Features

Closes

Test Plan

codecov bot commented Feb 8, 2021 • edited Loading

Codecov Report

Choose a reason for hiding this comment

damirka Feb 9, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

damirka Feb 9, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

damirka Feb 9, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

damirka Feb 9, 2021 • edited Loading

Choose a reason for hiding this comment

codecov bot commented Feb 8, 2021 •

edited

Loading

damirka Feb 9, 2021 •

edited

Loading

damirka Feb 9, 2021 •

edited

Loading

damirka Feb 9, 2021 •

edited

Loading

damirka Feb 9, 2021 •

edited

Loading