search

KyleMit / eleventy-plugin-embed-tweet

A plugin for embedding tweets on the server side during build time
https://www.npmjs.com/package/eleventy-plugin-embed-tweet
25 stars 4 forks source link

Use less generic env variables #3

Open nhoizey opened 4 years ago

nhoizey commented 4 years ago

The environment variable names are really generic: https://github.com/KyleMit/eleventy-plugin-embed-tweet/blob/master/twitter.js#L56-L69

I have keys for multiple services in my .env file, so it would be better having a more distinctive name for these for Twitter.

Here are the names in Twitter's UI: image

I suggest using these names, so follow Twitter naming:

current name   new name
TOKEN TWITTER_ACCESS_TOKEN
TOKEN_SECRET TWITTER_ACCESS_TOKEN_SECRET
CONSUMER_KEY TWITTER_API_KEY
CONSUMER_SECRET TWITTER_API_SECRET_KEY

If you agree with this, I can make a PR to use new names AND still support current ones for compatibility.

KyleMit commented 4 years ago

I like this change. We'll have to either support the old keys, with priority to new names and then fallback to older names, or revision as a breaking change. Not sure how many people are genuinely using this repo - and it's not a big lift to rename some properties. I wonder if we could code mod them over, but there just doesn't seem to be enough there to want to have users run a script to fix stuff.

nhoizey commented 4 years ago

I think you could support older names for a while, with a message in the console to show they're deprecated.