Open chshersh opened 1 year ago
@DukeManh I created this issue as a follow-up to your improvements in #122. You don't need to implement this issue (unless you want to), someone else could contribute the improvements 🙂 You've created the mechanism but in this issue I propose to use it in even more places.
Hi @chshersh I would like to work on this issue. Can you please assign it on my name?
Hi @ChellappanRajan 👋🏻 Sure, go for it 👍🏻
Custom decoding errors were implemented in the following PR:
122
This issue is about some minor improvements to the implementation so they can be done separately:
[ ] Move
Value::String("some value")
intoconst EXPECTED_STRING
orSTRING_TYPE
. It's a hack to reusetoml::Value
for types of expected values. We can move them into top-level constants for easier reuse in the future:https://github.com/chshersh/tool-sync/blob/f345444a548d215c2b560207b1ef031d18af28e2/src/config/toml.rs#L106
[ ] Return error when
asset_name
is not a table. Currently we return emptyAssetName
but we should provide a custom error when it's something likeasset_name = "x86_64_unknown_linux_musl"
. Implement a similar constant to the previous task for stringhttps://github.com/chshersh/tool-sync/blob/f345444a548d215c2b560207b1ef031d18af28e2/src/config/toml.rs#L163-L168
[ ] Return error when
str_by_key
doesn't seeString
. The function returnsOption<String>
. Its type should be changed toResult<Option<String>, DecodeError>
and its name should be changed tooptional_str_by_key
. And this function should throw an error when it sees something besides string.https://github.com/chshersh/tool-sync/blob/f345444a548d215c2b560207b1ef031d18af28e2/src/config/toml.rs#L184-L186
[ ] Throw
DecodeError
when iterating through the map. We expect all tools to be tables. So we should iterate through the map, filter out expected keys (proxy
andstore_directory
) and throw an error otherwisehttps://github.com/chshersh/tool-sync/blob/f345444a548d215c2b560207b1ef031d18af28e2/src/config/toml.rs#L124-L126