Rv crate #190
Rv crate #190
Conversation
src/add.rs
Outdated
| @@ -5,6 +5,7 @@ use toml_edit::{Array, DocumentMut, Formatted, Value}; | |||
|
|
|||
| use crate::{config::ConfigLoadError, Config}; | |||
|
|
|||
| /// Reads in the config to a toml_edit DocumentMut if it is a valid config file | |||
| pub fn read_and_verify_config(config_file: impl AsRef<Path>) -> Result<DocumentMut, AddError> { | |||
There was a problem hiding this comment.
That's the kind of function that really should not be open public
There was a problem hiding this comment.
This function was already pub-ed because its called from main. Should I move it to internal?
There was a problem hiding this comment.
This leaks internal stuff (toml editing). You should have add_packages take the path to the config file and call this fn inside add_packages (or in practice you don't even really need that fn)
| @@ -139,6 +142,7 @@ impl DiskCache { | |||
| self.root.join("urls").join(encoded) | |||
| } | |||
|
|
|||
| /// Get the path to the cloned git repo | |||
There was a problem hiding this comment.
If the comment is redundant with the function name, you can #![allow(missing_docs)] rather than repetitive comments
src/cache/disk.rs
Outdated
| @@ -168,6 +172,7 @@ impl DiskCache { | |||
| (path, false) | |||
| } | |||
|
|
|||
| /// Get the paths to a package in the cache | |||
src/cache/info.rs
Outdated
| @@ -54,6 +55,7 @@ pub struct CacheInfo { | |||
| } | |||
|
|
|||
| impl CacheInfo { | |||
| /// Create a new cache info object | |||
src/cli/context.rs
Outdated
| pub lockfile: Option<Lockfile>, | ||
| /// the R command line, containing the path to the selected R version |
There was a problem hiding this comment.
Yeah that's exactly what we don't want in practice
| } | ||
|
|
||
| pub fn add_packages(config_doc: &mut DocumentMut, packages: Vec<String>) -> Result<(), AddError> { | ||
| fn add_pkgs_to_config(config_doc: &mut DocumentMut, packages: Vec<String>) -> Result<(), AddError> { |
There was a problem hiding this comment.
I think the read_and_verify_config was implemented from this convo #107 (comment).
I abstracted one level so that it keeps a fxn taking it a DocumentMut, but can change it to whatever you feel is best
For other rv ecosystem tooling, elements of rv need to be made public