Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mandrill key should not have to be an environment variable #8

Open
RobinClowers opened this issue Dec 17, 2015 · 5 comments
Open

Mandrill key should not have to be an environment variable #8

RobinClowers opened this issue Dec 17, 2015 · 5 comments

Comments

@RobinClowers
Copy link
Contributor

Requiring the key to be set as in env does not seem to be idiomatic for elixir apps, it would be great if you could use the mix config system. You could still default the value what's in env, but fall back to using the config value defined by mix.

@slogsdon
Copy link
Owner

@RobinClowers I believe the documentation just needs to be updated. This was previously added: https://github.com/slogsdon/mandrill-elixir/blob/master/lib/mandrill.ex#L49-L52. It pulls the key from Application environment variable, but if nil, it reaches out to System environment variable.

As a note, this should probably be updated as well to remove the || operator, but it should work as is.

@RobinClowers
Copy link
Contributor Author

That's great news, I'll send you a pr to update the docs this weekend.

@RobinClowers
Copy link
Contributor Author

When you say remove the || operator, you mean remove the option to set it through the environment variable? I still think that's a nice fallback, in some cases it's more convenient to set it that way.

@slogsdon
Copy link
Owner

@RobinClowers, Here's what I meant by that statement. Right now this is essentially what the code looks like:

Application.get_env(:mandrill, :key) || System.get_env("MANDRILL_KEY")

When I said that the || operator should probably be removed, I was suggesting that the use of that operator is unnecessary since Application.get_env/3 is a better alternative. Using that, the above code would look something like this:

Application.get_env(:mandrill, :key, System.get_env("MANDRILL_KEY")

Looking back, I could have been clearer on that, so I apologize for any confusion.

@RobinClowers
Copy link
Contributor Author

Awesome, thanks for the explanation, that makes total sense.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants