Tool Scrubber Graduation PR for V6 #1174
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
0o-de-lally
reviewed
Oct 5, 2022
| @@ -309,15 +309,22 @@ pub fn write_account_json( | |||
| let keys = KeyScheme::new(&wallet); | |||
There was a problem hiding this comment.
write_account_json should return Result
0o-de-lally
reviewed
Oct 5, 2022
| @@ -9,7 +9,7 @@ use crate::prelude::app_config; | |||
| use diem_types::transaction::SignedTransaction; | |||
| use diem_wallet::WalletLibrary; | |||
| use ol_types::{account::ValConfigs, pay_instruction::PayInstruction}; | |||
| use std::path::PathBuf; | |||
| use std::{path::PathBuf, process::exit}; | |||
|
|
|||
| /// Creates an account.json file for the validator | |||
| pub fn write_manifest( | |||
0o-de-lally
reviewed
Oct 5, 2022
| @@ -41,6 +41,11 @@ impl Runnable for ValConfigCmd { | |||
| None, | |||
| ); | |||
|
|
|||
| let val_cfg = match val_cfg_res { | |||
| Ok(cfg) => cfg, | |||
| Err(error) => panic!("Could not create validator config: {:?}", error), | |||
There was a problem hiding this comment.
remove panic! for exit()
0o-de-lally
reviewed
Oct 5, 2022
| @@ -85,12 +85,11 @@ impl ValConfigs { | |||
| vfn_ip_address: Ipv4Addr, | |||
| autopay_instructions: Option<Vec<PayInstruction>>, | |||
| autopay_signed: Option<Vec<SignedTransaction>>, | |||
| ) -> Self { | |||
| ) -> Result<ValConfigs, anyhow::Error> { | |||
There was a problem hiding this comment.
Result<ValConfigs...
Result<Self...
0o-de-lally
reviewed
Oct 5, 2022
| @@ -197,8 +197,8 @@ impl PayInstruction { | |||
| uid = &self.uid.unwrap(), | |||
| percent_balance = *&self.value_move.unwrap() as f64 /100f64, | |||
There was a problem hiding this comment.
Another opportunity to refactor.
0o-de-lally
reviewed
Oct 5, 2022
| @@ -197,8 +197,8 @@ impl PayInstruction { | |||
| uid = &self.uid.unwrap(), | |||
0o-de-lally
reviewed
Oct 5, 2022
| @@ -197,8 +197,8 @@ impl PayInstruction { | |||
| uid = &self.uid.unwrap(), | |||
| percent_balance = *&self.value_move.unwrap() as f64 /100f64, | |||
| times = times, | |||
| note = &self.note.clone().unwrap(), | |||
| epoch_ending = &self.end_epoch.unwrap(), | |||
| note = if let Some(n) = &self.note { n } else {""}, // the note is optional for the message | |||
0o-de-lally
reviewed
Oct 5, 2022
| // let mut config_toml = path.to_str().unwrap().to_owned()).expect("could not parse app config from file"); | ||
|
|
||
| let cfg_path = match path { |
There was a problem hiding this comment.
.unwrap_or(dirs::home_dir().expect("WTF where's home").join(".0L").join("0L.toml"));
// this unwrap is likely safe in this case. nothing to recover from.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I refactored 10 unwrap commands and 10 clone commands in various files in the 0l folder. I noticed some automatically updated binaries from the folder "DPNFramework" made it into this PR, too, maybe due to my test activities. I did not make code changes in those. I hope that's not a problem.
Motivation
Tool Scrubber Graduation
Test Plan
I complied, spun up a node and executed the shuffle e2e test. The expected result of the first test passing and the remaining 3 tests failing was observed:
running 4 tests from ./e2e/message.test.ts
Test Assert ...
------- output -------
{ chain_id: 4, ledger_version: "96", ledger_timestamp: "1664728533581067" }
----- output end -----
Test Assert ... ok (14ms)
Ability to set message ... FAILED (38ms)
Ability to set NFTs ... FAILED (11ms)
Advanced: Ability to set message from nonpublishing account ... FAILED (8ms)