-
Notifications
You must be signed in to change notification settings - Fork 351
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
Moved sensitive information to .env #144
base: master
Are you sure you want to change the base?
Conversation
you dropped youtube-node down to 1.2 from 1.3? but overall this looks good. heading out to comiccon rn but will take a look tomorrow |
Support for getting credentials from environment variables is definitely nice, but can we make this backwards compatible so it isn't a breaking change for people with the bot already set up? |
I think a small script which delete the const oldCredentials = require('auth.json');
let credsList = Object.keys(oldCredentials);
let dotEnvContent = '';
credsList.forEach(cred => {
dotEnvContent += cred.toUpperCase()+'= '+oldCredentials[cred];
});
require('fs').writeFileSync('.env', dotEnvContent);
console.log('auth.json file has been moved to .env file!'); |
huh just remmebered about this. i ended up implementing a much hackier solution to get stuff to work in heroku 😭 ill try to rebase this and get it to work with my changes. I think ill keep my changes for now that work both with json and env tho :P |
This code looks cleaner as it doesn't require you to write all the credentials all by yourself in a .env file. But the problem is while working with config.json files we can assign numeric values to a key, but in the case of the .env file, everything is converted to a string. |
I see a lot of improvements for this bot,
some packages are required in multiple places instead of 1, unnecessary try-catch blocks.
Sensitive information in JSON instead of dotenv module.