-
Notifications
You must be signed in to change notification settings - Fork 110
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
remote-update #200
base: master
Are you sure you want to change the base?
remote-update #200
Conversation
Codecov Report
@@ Coverage Diff @@
## master #200 +/- ##
==========================================
- Coverage 88.07% 88.07% -0.01%
==========================================
Files 110 110
Lines 9500 9499 -1
==========================================
- Hits 8367 8366 -1
Misses 1133 1133
Continue to review full report at Codecov.
|
Cargo.toml
Outdated
@@ -51,6 +51,9 @@ safemem = { version = "0.3.3" } | |||
serde = { version = "1.0", features = ["derive"] } | |||
serde_json = { version = "1.0" } | |||
tiny-keccak = { version = "1.4" } | |||
reqwest = { version = "0.10.6", features = ["json"] } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: please order the imports alphabetically
wagyu/lib.rs
Outdated
@@ -9,3 +9,4 @@ pub extern crate wagyu_zcash as zcash; | |||
|
|||
#[cfg_attr(tarpaulin, skip)] | |||
pub mod cli; | |||
pub mod remote_update; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: please add a newline after
wagyu/main.rs
Outdated
@@ -27,15 +30,24 @@ fn main() -> Result<(), CLIError> { | |||
EthereumCLI::new(), | |||
MoneroCLI::new(), | |||
ZcashCLI::new(), | |||
SubCommand::with_name("update").about("Auto update to latest version"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: can you change the description to read Update wagyu to the latest version
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you formalize this into an UpdateCLI
struct, so that the command's functionality is encapsulated under the parse
and print
model we have defined?
This will enable future extensibility of the update command without introducing complexity outside of the Struct's scope.
wagyu/remote_update.rs
Outdated
Ok(_) => println!("Update finished."), | ||
Err(err) => println!("{}\nUpdate failed.", err) | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: please add a newline
wagyu/remote_update.rs
Outdated
println!("You are using the latest version."); | ||
} | ||
} | ||
Err(_) => println!("Please check the internet connection for the automatic version update.") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change this to read You are in offline mode.
wagyu/remote_update.rs
Outdated
use tokio::runtime::Runtime; | ||
use reqwest; | ||
use serde_json; | ||
use serde_json::Value; | ||
use self_update; | ||
|
||
use clap::{crate_version}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use tokio::runtime::Runtime; | |
use reqwest; | |
use serde_json; | |
use serde_json::Value; | |
use self_update; | |
use clap::{crate_version}; | |
use clap::{crate_version}; | |
use reqwest; | |
use self_update; | |
use serde_json; | |
use serde_json::Value; | |
use tokio::runtime::Runtime; |
wagyu/remote_update.rs
Outdated
println!("New version {} available.", version); | ||
return version; | ||
} else { | ||
println!("You are using the latest version."); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change this to read You are on the latest version.
wagyu/main.rs
Outdated
]) | ||
.set_term_width(0) | ||
.get_matches(); | ||
|
||
let latest_version = remote_update::version_check(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be implemented as part of a UpdateCLI::version_check()
wagyu/main.rs
Outdated
match arguments.subcommand() { | ||
("bitcoin", Some(arguments)) => BitcoinCLI::print(BitcoinCLI::parse(arguments)?), | ||
("ethereum", Some(arguments)) => EthereumCLI::print(EthereumCLI::parse(arguments)?), | ||
("monero", Some(arguments)) => MoneroCLI::print(MoneroCLI::parse(arguments)?), | ||
("zcash", Some(arguments)) => ZcashCLI::print(ZcashCLI::parse(arguments)?), | ||
("update", Some(_)) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should adhere to the same *CLI
parse and print convention
wagyu
is called, it checks for updates.wagyu
is called in an offline setting, it detects if it is offline and doesn't check for an update, and indicates to the user that it is in offline mode.wagyu update
is called, it fetches the latest release if it's outdated.