Make necessary traits _pub_ and add basic error handling #7
Conversation
config/src/lib.rs
Outdated
| @@ -568,7 +574,7 @@ mod tests { | |||
| OsString::from("--testmultiple"), | |||
| OsString::from("3"), | |||
| ]; | |||
| let config: TestConfig = ConfigBuilder::build(command_line_args, &env_var_name) | |||
| let config: TestConfig = *ConfigBuilder::build(command_line_args, &env_var_name) | |||
There was a problem hiding this comment.
All of these derefs can go if you make config type Box, it'll be dereferenced automatically later so it should compile.
That said: See commment about Box earlier.
config/src/lib.rs
Outdated
| fn parse_values(parsed_values: HashMap<ConfigOption, Option<Vec<String>>>) -> Self; | ||
| fn parse_values( | ||
| parsed_values: HashMap<ConfigOption, Option<Vec<String>>>, | ||
| ) -> Result<Box<Self>, anyhow::Error>; |
There was a problem hiding this comment.
I don't think you need any of these Box things as long as Configurable is Sized. I'm not 100% sure but removing all of those and running the tests works.
I also think that anyhow shouldn't appear in the API (but I'm not sure about that tbh.)
There was a problem hiding this comment.
I think trying to return the trait Error from these functions in the result was what got me started down the Box<..> route, so using anyhow::error removed the need for Boxes all over the place. I agree that it would be nicer to define a proper error type and have those error be part of the api though.
This PR makes the traits that are intended for external usage public, which was forgotten before.
Also, some functions are refactored to be able to return errors as part of a Result<..,..> type.