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

Update Configuration instructions in README.md #457

Merged
merged 2 commits into from Feb 5, 2019

Conversation

annaminton
Copy link
Contributor

As pointed out in #456, you don't need the function and can set the api_key directly.

@coveralls
Copy link

coveralls commented Feb 5, 2019

Coverage Status

Coverage remained the same at 83.333% when pulling 5272ec5 on annaminton:patch-1 into 24c2f93 on code-corps:master.

README.md Outdated
@@ -63,7 +63,7 @@ It's possible to use a function or a tuple to resolve the secret:
```ex
config :stripity_stripe, api_key: {MyApp.Secrets, :stripe_secret, []}
# OR
config :stripity_stripe, api_key: fn -> System.get_env("STRIPE_SECRET") end
config :stripity_stripe, api_key: System.get_env("STRIPE_SECRET")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's possible to use a function or a tuple to resolve the secret:

👋 @annaminton! Really appreciate the PR. So this is under an alternative configuration option. I asked @monorkin if he ended up using this option to set a config option just to make sure there are no issues.

The way you outlined here is the "main" way most people set their api_key, which is also documented above (partially). Perhaps we could change this PR to update the above config :stripity_stripe, api_key: "YOUR SECRET KEY" with what you have here? Lmk what you think.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are correct. I apologize for not reading the README more carefully. I honestly skipped over that part and went straight for the alternate configuration after spotting System.get_env.

I do think the vast majority of users will be setting their :stripe_secret via an environment variable so updating the "main" example probably makes sense.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've made that change. Take a look and let me know if you think this is adding any value. 🙌

Copy link
Collaborator

@snewcomer snewcomer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wohoo! Nice job and thank you!

@snewcomer snewcomer merged commit f5e92fc into beam-community:master Feb 5, 2019
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

Successfully merging this pull request may close these issues.

None yet

3 participants