-
Notifications
You must be signed in to change notification settings - Fork 24
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
Recursive Dependencies #136
Conversation
…ps which can install the dependencies of that dependency recursively
src/dependency_downloader.rs
Outdated
@@ -287,6 +285,49 @@ fn transform_git_to_http(url: &str) -> String { | |||
} | |||
} | |||
|
|||
fn check_recursiveness(dependency: &Dependency) -> Result<()> { |
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 function is not a check (which I would expect to have no side effects). A better name would be install_subdependencies
src/dependency_downloader.rs
Outdated
.stdout(Stdio::piped()) | ||
.stderr(Stdio::piped()); | ||
|
||
let status = result.status().unwrap(); |
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.
Avoid unwrap
, better to report the error instead of panicking? If don't want to return an error because the program must absolutely panic here, then at least use expect()
with a descriptive message. Same comment on line 308
src/dependency_downloader.rs
Outdated
@@ -117,6 +119,10 @@ pub async fn download_dependency( | |||
} | |||
} | |||
}; | |||
|
|||
if recursive_deps { | |||
check_recursiveness(dependency).unwrap(); |
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.
No unwrap here! Just propagate the error with question mark
Solves the recursive dependencies.
if you set --recursive-deps when you run install or update or you set the config
[soldeer]
recursive-deps = true
Soldeer will install automatically the submodule dependencies and soldeer dependencies of the dependency you want to install.
Closes #115