Skip to content
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

Use structopt for command line arguments #40

Merged
merged 3 commits into from
Apr 23, 2019

Conversation

nbaksalyar
Copy link
Contributor

Using this library gives us more flexibility in ways users can configure the options provided by the library.
This PR also includes some changes in directories/config file loader handling (e.g., it also takes mobile platforms into account).

It makes the library usage more flexible: with structopt flatten we can
create CLI apps which can inherit configuration options from quic-p2p.
…atforms

It is not supported on Android/iOS and embedded platforms so we don't need it.
Also add placeholders for project_dir.
@@ -13,7 +13,7 @@ use crate::error::Error;
use serde::de::DeserializeOwned;
use serde::Serialize;
use std::fs::File;
use std::io::{self, BufReader, BufWriter};
use std::io::{BufReader, BufWriter};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why was this necessary

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not used on non-T1/mobile platforms (see the project_dir impl below)

src/config.rs Outdated
pub idle_timeout_msec: Option<u64>,
/// Interval to send keep-alives if we are idling so that the peer does not disconnect from us
/// declaring us offline. If none is supplied we'll default to the documented constant.
///
/// The interval is in milliseconds. A value of 0 disables this feature.
#[structopt(long)]
pub keep_alive_interval_msec: Option<u32>,
/// Path to our TLS Certificate. This file must contain `SerialisableCertificate` as content
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah this is not correct - can we pls change it to say it's not a path we are asking but the certificate itself ?

@nbaksalyar nbaksalyar marked this pull request as ready for review April 23, 2019 10:08
@ustulation ustulation merged commit b31c471 into maidsafe:master Apr 23, 2019
@nbaksalyar nbaksalyar deleted the structopt branch April 23, 2019 12:24
@nbaksalyar nbaksalyar mentioned this pull request Apr 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants