asonix / release-manager

A utility to manage releasing rust programs for multiple platforms
1 stars 1 forks source link

Refactor String -> Enum conversions to use TryFrom #4

Closed asonix closed 6 years ago

asonix commented 6 years ago

Two places in the code (specifically for the OS and Arch enums, we have extended match structures that convert a given &str into an enum discriminant. We can clean up the logic here by replacing these with something along the lines of:

if let Ok(value) = arch_str.try_into() {
    // call_things_on(value)
}

// somewhere else in the code
impl TryFrom<&str> for Arch {
    fn try_from(...) { ... }
}
ghost commented 6 years ago

Hello, I have migrated the match structures into the TryFrom implementation, Would you accept the following in place of your reccomended if statement?

      for (os, value) in self.config.iter() {
            let opsys = OS::try_from(os.as_ref());
            build_os(&mut targets, opsys.expect(&format!("{}, is not a valid Operating System!",os)), value);
        }
asonix commented 6 years ago

Try not to use 'expect' since we don't really want to have our program panic in these cases. Printing a message and ignoring the invalid target is probably better

ghost commented 6 years ago

Copy that, I'll make the modifications :>

Thanks, Aidan

On Dec 14, 2017 6:11 PM, "Riley Trautman" notifications@github.com wrote:

Try not to use 'expect' since we don't really want to have our program panic in these cases. Printing a message and ignoring the invalid target is probably better

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/asonix/release-manager/issues/4#issuecomment-351893309, or mute the thread https://github.com/notifications/unsubscribe-auth/AIKFgJqUC4Mw5UoZcHYf1HvKqliGCBfMks5tAdU4gaJpZM4RBWim .

ghost commented 6 years ago

Okay, Instead of 'expect' it now checks the result with 'is_err' before reaching inside it with 'unwrap.'

asonix commented 6 years ago

Nice, thanks!