-
-
Notifications
You must be signed in to change notification settings - Fork 18
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
[#57] Adds install command to install known tools #89
Conversation
Im not sure what tests can be added to make sure everything works properly. |
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.
Really nice! I'm surprised how easy you implemented this command! 🤯
I have a few suggestions to improve the code and some ideas for future improvements but this MVP is already good enough to be shipped with the v0.2.0
release after a few code changes 👍🏻
src/lib.rs
Outdated
Command::Install { name } => { | ||
if let Some(tool_info) = lookup_tool(&name) { | ||
if let Ok(mut tool) = toml::parse_file(&config_path) { | ||
let tool_btree: BTreeMap<String, ConfigAsset> = | ||
BTreeMap::from([(name, tool_info.into())]); | ||
tool.tools = tool_btree; | ||
sync(tool); | ||
} | ||
} | ||
} |
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.
Ideas for future improvements (can be done as separate issues):
- Update the config to add a new tool (so it later will be updated with
sync
) - Support all
ConfigAsset
fields as arguments to theinstall
commandj - Allow downloading the tool from the parsed config if it's already specified in there
I think we can add similar integration tests in the |
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.
That looks great 👏🏻
Resolves #57
This pr adds an
isntall
subcommand that lets the user install any of the hardcoded tools. As a bonus it also implements theFrom
trait to convert fromToolInfo
ToConfigAsset
.Additional tasks