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

Prefix environment variables with "TWITTER_" #6

Closed
wants to merge 3 commits into from

Conversation

bzg
Copy link
Contributor

@bzg bzg commented Apr 22, 2019

I find ACCESS_TOKEN may conflict with environment variables for other applications/services.

@chbrown
Copy link
Owner

chbrown commented Apr 23, 2019

While I generally appreciate namespacing, this is liable to break things, beginning with my Travis CI tests, and ending with any end-users (including me) who call (env->UserCredentials) or (env->AppCredentials) without arguments.

Perhaps I should have chosen different names when forking this library and bumping to v3, but I've run the gauntlet of using several different Twitter clients in multiple languages, and these names seemed to me to be the consensus (though by no means universal).

I'll think on it a bit longer, but I don't think I'll merge this PR, sorry! :/

(Looking at your changes, I see you realize that (env->*Credentials ...) optionally takes an argument, so this example is primarily for any other users having the same issue.)

You can achieve exactly what you want by replacing all instances in your codebase of:

(env->UserCredentials)  ; <- old way, uses default mapping without TWITTER_ prefixes

with:

; explicitly specify environment variables to use and how:
(env->UserCredentials {:consumer-key      "TWITTER_CONSUMER_KEY"
                       :consumer-secret   "TWITTER_CONSUMER_SECRET"
                       :user-token        "TWITTER_ACCESS_TOKEN"
                       :user-token-secret "TWITTER_ACCESS_TOKEN_SECRET"})

...and similarly for (env->AppCredentials):

(env->AppCredentials {:consumer-key      "TWITTER_CONSUMER_KEY"
                      :consumer-secret   "TWITTER_CONSUMER_SECRET"})

bzg added a commit to bzg/retweet that referenced this pull request Apr 24, 2019
@bzg
Copy link
Contributor Author

bzg commented Apr 24, 2019

Hi @chbrown, absolutely no problem in not merging, this was just a suggestion!

The solution you point to is actually good enough for me. I hope there won't be any confusion resulting from using different environment variables names in different apps (with or without the TWITTER_ prefix).

Thanks again for taking the time to handle this.

@bzg bzg closed this by deleting the head repository Oct 28, 2024
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.

2 participants