ratmice commented on code in PR #23797:
URL: https://github.com/apache/datafusion/pull/23797#discussion_r3646429473
##########
datafusion-cli/src/exec.rs:
##########
@@ -129,13 +130,15 @@ pub async fn exec_from_files(
pub async fn exec_from_repl(
ctx: &dyn CliSessionContext,
print_options: &mut PrintOptions,
+ history_file: Option<&Path>,
Review Comment:
I think it might be better instead of just adding an `Option<&Path>`,
to declare some struct like perhaps
```
#[non_exhaustive]
#[derive(Default)]
struct OtherOptions {
history_path: Option<PathBuf>,
}
```
Or manually implement default to default to ".history" here in the struct
than in the function.
Then if any new options are needed which have default values, we don't have
to change the abi again to add them, because they should just get picked up by
the non_exhaustive/default combo.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
