Feature/repo add with optionals #15
Conversation
|
Hi @andreclaudino, thanks for the PR! I'll read through this and leave some comments sometime in the next day :) |
| @@ -0,0 +1,116 @@ | |||
| use std::process::Command; | |||
|
|
|||
There was a problem hiding this comment.
I wonder if this file should be called repo_add_args instead of _optionals. Seems like it'd be more consistent with the other filenames
| ca_file: Option<String>, | ||
| cert_file: Option<String>, | ||
| key_file: Option<String>, |
There was a problem hiding this comment.
Prefer using std::path::PathBuf for file values
| pub fn set_ca_file<S: Into<String>>(mut self, value: S) -> Self { | ||
| self.ca_file = Some(value.into()); | ||
| self | ||
| } | ||
|
|
||
| pub fn set_cert_file<S: Into<String>>(mut self, value: S) -> Self { | ||
| self.cert_file = Some(value.into()); | ||
| self | ||
| } | ||
|
|
||
| pub fn set_key_file<S: Into<String>>(mut self, value: S) -> Self { | ||
| self.key_file = Some(value.into()); | ||
| self | ||
| } |
There was a problem hiding this comment.
These can use <P: Into<PathBuf>> instead
| } | ||
| } | ||
|
|
||
| impl Into<Command> for RepoAddArg { |
There was a problem hiding this comment.
This makes me realize, we should be using impl From<RepoAddArg> for Command. It is generally better to implement From than Into. We may want to change this for the other commands as well
|
@andreclaudino can you minimize this PR to only semantic changes in the code? It is easier to review when there is not the extra diff noise of moving code from one file to another. I saw you opened another PR for just breaking |
|
@nicholastmosher, You can just ignore this PR for a while. Evaluate the other, if is ok, I can just open a new PR with the complements. |
Add a funtion to allow repository add with the other command line parameters. Useful to add authenticated repositories