allan2 / dotenvy

A well-maintained fork of the dotenv crate
MIT License
664 stars 40 forks source link

Change misleading usage examples #37

Closed xiaoas closed 1 year ago

xiaoas commented 1 year ago

The example in README use dotenvy::dotenv feels miss-leading:

fn main() {
    dotenv().ok();

    for (key, value) in env::vars() {
        println!("{key}: {value}");
    }
}

calling .ok on dotenv(), a Result, turns it into a Option. This is not what most people might expect imo. Changing to some error handling might better serve the intention:

fn main() -> Result<(), dotenvy::Error>{
    dotenv()?; // or dotenv().unwrap();

    for (key, value) in env::vars() {
        println!("{key}: {value}");
    }
}
sonro commented 1 year ago

The reason for the .ok() is to ignore the error.

The .env file this loads might not be available on the deployed version of a user's app. The environment variables would usually be set elsewhere. By using this pattern no error is thrown if the file isn't found.

As some users may require a .env file, they have the option of using its result type.


You are right: there needs to be better documentation around this issue. It certainly doesn't feel very "rusty" to just .ok() an error like that.

allan2 commented 1 year ago

@xiaoas, thanks for reporting. It's been brought up a few times already that .ok() is not ideal.

I've replaced some .ok()s in the docs with .unwrap() but there are still a few instances left, including the README example. Feel free to open a PR.