-
Notifications
You must be signed in to change notification settings - Fork 3
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
Chain add command #51
base: old-main
Are you sure you want to change the base?
Conversation
.repos("cosmos", "chain-registry") | ||
.raw_file("88bde7fb534ed6f7c26c2073f57ec5135b470f56".to_string(), path) | ||
.await | ||
.map_err(|e| e.into()) | ||
.unwrap_or_else(|err| { | ||
panic!("executor exited with error: {}", 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.
Let's return an error instead,
ChainRegistryError::Request()
takes in an octocrab::Error
let status = response.status(); | ||
|
||
if status == 404 { | ||
panic!("Chain not found in registry") |
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.
Same here, ChainRegistryError::UnsupportedChain
is made for this
let mut vec = Vec::new(); | ||
vec.push(chain_info); | ||
|
||
if !chain_content.contains(chain_name) { |
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.
If the chain exists in the config we can avoid a call to github by putting a check for this before the async block.
let mut file = fs::OpenOptions::new() | ||
.append(true) | ||
.open(config_file) | ||
.expect("Could not open file"); |
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.
I think it will be a better UX if the list of chains is in alphabetical order like lens
has it, which appending can't accomplish.
I'd forgotten but there is actually an OcularCliConfig
struct for serializing/deserializing the config file already in config.rs
that can replace the Chains
struct in this file. You should be able to deserialize the file to it and edit the chains vec (and sort):
ocular/ocular_cli/src/config.rs
Lines 16 to 24 in 608351f
/// OcularCli Configuration | |
#[derive(Clone, Debug, Default, Deserialize, Serialize)] | |
#[serde(deny_unknown_fields)] | |
pub struct OcularCliConfig { | |
/// Chain related config, not read in from file | |
pub default_chain: String, | |
/// Locally cached chains | |
pub chains: Vec<ChainInfo>, | |
} |
closes #32