-
Notifications
You must be signed in to change notification settings - Fork 259
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
Replace tempdir with tempfile #2760
Conversation
fn run( | ||
self, | ||
trigger_app: TriggerApp<Self>, | ||
) -> impl Future<Output = anyhow::Result<()>> + Send; | ||
fn run(self, trigger_app: TriggerApp<Self>) -> impl Future<Output = anyhow::Result<()>> + Send; |
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.
Sneaky unrelated rustfmts 🕵️
Signed-off-by: Lann Martin <[email protected]>
Signed-off-by: Lann Martin <[email protected]>
Signed-off-by: Lann Martin <[email protected]>
@@ -312,7 +312,7 @@ impl<T: Trigger> TriggerAppBuilder<T> { | |||
let use_gpu = true; | |||
let state_dir = match options.state_dir { | |||
// Make sure `--state-dir=""` unsets the state dir | |||
Some(s) if s.trim().is_empty() => None, | |||
Some("") => None, |
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.
Sneaky lint fix...
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.
This isn't equivalent, no? Some(" ")
would match the first but not the second.
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.
Ah sorry I blindly obeyed clippy. That said, --state-dir " "
seems more likely to be an accident...
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.
Though it now occurs to me that its perfectly valid to create a directory named " "
...
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! Thanks!
tempdir
is abandoned