-
-
Notifications
You must be signed in to change notification settings - Fork 3
feat: add progress reporting #376
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
Conversation
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.
Really nice. Here's some initial feedback.
I noticed a lot of formatting changes, but I couldn't tell if it was new or old styling.
I also see pre-commit failed, so could be related.
Co-authored-by: Nick <10092581+NickLarsenNZ@users.noreply.github.com>
Co-authored-by: Nick <10092581+NickLarsenNZ@users.noreply.github.com>
```shell cargo +nightly-2025-01-15 fmt --all ```
…d use span filtering toc ontrol indicatif output NOTE: Use `fields(indicatif.pb_show = true)` on instrumented functions that should have progress reporting
@@ -303,14 +306,15 @@ fn describe_cmd( | |||
} | |||
} | |||
|
|||
#[instrument(skip(cli, stack_list, transfer_client))] | |||
#[instrument(skip(cli, stack_list, transfer_client), fields(indicatif.pb_show = true))] |
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.
Note to self, we should skip_all
here, since args ends up looking so bad in the logs (since it uses the Debug impl). Nothing related to this PR though.
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.
LGTM
Description
Implementation for #153
Adds various progress indications/visualizations to
stackablectl
subcommand outputs.Some explanation regarding
println!
/eprintln!
changes in this PR:When indicatif is active, it needs to control how text is printed. So instead of printing straight to
stdout
/stderr
viaprintln!
/eprintln!
, we useindicatif_println!
/indicatif_eprintln!
so it can deal with it while rendering progress bars. If it is not active, the indicatif macros just usestdout
/stderr
.Definition of Done Checklist