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

Adds debug arg to the CLI #71

Merged
merged 3 commits into from
Aug 24, 2023
Merged

Conversation

sr-gi
Copy link
Member

@sr-gi sr-gi commented Aug 18, 2023

Debug logs can be turned in using --debug.

Also fixes pending logs.

Simplifies the JoinSet management to have a single global sets of tasks
Total time can be set via cmd using `--total-time`. If no time is provided,
the simulation is run forever
Debug logs can be turned in using `--debug`.

Also fixes pending logs
@sr-gi sr-gi changed the title Debug arg Adds debug arg to the CLI Aug 18, 2023
Copy link
Contributor

@carlaKC carlaKC left a comment

Choose a reason for hiding this comment

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

LGTM - possibly want to update log config to something like --log-level=value so that we can set any logging level but already a big improvement!

@@ -233,6 +233,7 @@ impl Simulation {
self.activity.len(),
self.nodes.len()
);
let mut tasks = JoinSet::new();
Copy link
Contributor

Choose a reason for hiding this comment

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

noice 🥇

@@ -16,19 +16,35 @@ struct Cli {
config: PathBuf,
#[clap(long, short)]
total_time: Option<u32>,
#[clap(long, short)]
debug: bool,
Copy link
Contributor

Choose a reason for hiding this comment

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

Bool config option vs passing in a string that allows enabling other values like trace?

}

#[tokio::main]
async fn main() -> anyhow::Result<()> {
let cli = Cli::parse();
Copy link
Collaborator

Choose a reason for hiding this comment

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

would we also want to read in RUST_LOG environment at this point?

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think I follow

@carlaKC carlaKC merged commit f57ff45 into bitcoin-dev-project:main Aug 24, 2023
2 checks passed
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.

3 participants