-
Notifications
You must be signed in to change notification settings - Fork 1
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
NEXT-37318 - copy default profiles from executable #4
NEXT-37318 - copy default profiles from executable #4
Conversation
4deac8c
to
a5a1bde
Compare
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.
Overall I like the implementation of this new command, great work 👍 .
But I'm not really sure if we want this build script, we can also talk about it, just reach out 🙂
Regarding you question: I don't know of any constants for the build script, just this documentation: So I think you would need to hard code them... |
a5a1bde
to
4abd90c
Compare
4abd90c
to
3352229
Compare
@@ -15,6 +16,25 @@ mod cli; | |||
mod config_file; | |||
mod data; | |||
|
|||
const PROFILES: &[(&str, &str)] = &[ |
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.
Do you want to try to write a unit test that ensures that all files in ./profiles
folder are actually defined in this array?
https://doc.rust-lang.org/book/ch11-01-writing-tests.html
You could also extend it to parse these profiles and see if that produces errors.
If you don't, that's also fine and I try to cover it together with my "test coverage" ticket 🙂
let mut dir_path = PathBuf::from("./profiles"); | ||
|
||
if path.is_some() { | ||
let path = path.unwrap(); | ||
|
||
if !path.is_dir() && !force { | ||
eprintln!("Path is not a directory: {:?}", path); | ||
return; | ||
} | ||
|
||
dir_path = path; | ||
} |
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 little more idomatic rust 🤓
let mut dir_path = PathBuf::from("./profiles"); | |
if path.is_some() { | |
let path = path.unwrap(); | |
if !path.is_dir() && !force { | |
eprintln!("Path is not a directory: {:?}", path); | |
return; | |
} | |
dir_path = path; | |
} | |
let Some(dir_path) = path else { | |
PathBuf::from("./profiles") | |
}; | |
if !dir_path.is_dir() && !force { | |
eprintln!("Path is not a directory: {:?}", path); | |
return; | |
} |
@MalteJanz, is there a way to better organize environment constants? It feels a bit awkward to hardcode things like this or this.
You might need to update your
settings.json
in VS Code to clear an analyzer error found here: link.