-
Notifications
You must be signed in to change notification settings - Fork 893
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
Tweaks from derived clap parser review #3834
Conversation
@@ -1515,12 +1518,6 @@ fn set_auto_self_update(cfg: &mut Cfg, auto_self_update_mode: &str) -> Result<ut | |||
Ok(utils::ExitCode(0)) | |||
} | |||
|
|||
#[cfg_attr(feature = "otel", tracing::instrument(skip_all))] |
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.
A question for @rbtcollins: when do we decide that we need to instrument a function?
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.
Thank you so much for the work! I do think this has addressed many concerns I have had in terms of code readability :)
PS: There seems to be some comptime errors to be addressed, but this doesn't influence my approval.
Follow-up from #3596. I think none of this is really things that originated in that PR, just trying to improve the codebase as I notice stuff.