We recommend upgrading to the latest Google Chrome or Firefox.
GitHub is home to over 20 million developers working together to host and review code, manage projects, and build software together.
Interesting approach moving that out to a shared module! I think that's a first for a hubot-scripts thing.
Perhaps instead of breaking things by changing the expected variable, could still use the HUBOT_TWITTER_ACCESS_TOKEN_KEY and HUBOT_TWITTER_ACCESS_TOKEN_SECRET, where those would be the 'default credentials'.
|
Interesting approach moving that out to a shared module! I think that's a first for a hubot-scripts thing. Perhaps instead of breaking things by changing the expected variable, could still use the HUBOT_TWITTER_ACCESS_TOKEN_KEY and HUBOT_TWITTER_ACCESS_TOKEN_SECRET, where those would be the 'default credentials'. |
| - access_token_secret: process.env.HUBOT_TWITTER_ACCESS_TOKEN_SECRET | ||
| + auth = twitterConfig.defaultCredentials() | ||
| + unless auth | ||
| + console.log "Please set HUBOT_TWITTER_CONSUMER_KEY_<USERNAME> and HUBOT_TWITTER_CONSUMER_SECRET_<USERNAME>." |
I would move this into the robot.respond block, and make it do msg.send. For users of the system, the command would just fail, and it'd take some digging into the logs to find the output for this little console.log.
It might make sense to add a helper to twitterConfig that takes a msg object so it can output what goes wrong. Like:
auth = twitterConfig.defaultCredentials()
unless auth.checkEnvironment(msg)
returnThen have checkEnvironment do the msg.send for errors, and return false if something isn't set.
I would move this into the robot.respond block, and make it do msg.send. For users of the system, the command would just fail, and it'd take some digging into the logs to find the output for this little console.log.
It might make sense to add a helper to twitterConfig that takes a msg object so it can output what goes wrong. Like:
auth = twitterConfig.defaultCredentials()
unless auth.checkEnvironment(msg)
returnThen have checkEnvironment do the msg.send for errors, and return false if something isn't set.
| -# "github" : { "access_token" : "", "access_token_secret" : ""} | ||
| -# } | ||
| +# See here for environment variable descriptions: | ||
| +# https://github.com/github/hubot-scripts/tree/master/src/twitter-config.js |
Ok, I think all has been addressed, minus the checkEnvironment() thing - I just couldn't think of a super clean way to handle it.
|
Ok, I think all has been addressed, minus the |
I want to try this out locally, but really like the direction this is going!
|
I want to try this out locally, but really like the direction this is going! |
I'm pretty sure we'd need separate HUBOT_TWITTER_CONSUMER_KEY and HUBOT_TWITTER_CONSUMER_KEY per account, because each account would have it's own OAuth on dev.twitter.com for accessing things.
|
I'm pretty sure we'd need separate |
@afeld yeah, this would need to track consumer key & secret per twitter account in addition to the access key & secret.
|
@afeld yeah, this would need to track consumer key & secret per twitter account in addition to the access key & secret. |
| @@ -8,8 +8,8 @@ | ||
| # Configuration: | ||
| # HUBOT_TWITTER_CONSUMER_KEY | ||
| # HUBOT_TWITTER_CONSUMER_SECRET | ||
| -# HUBOT_TWITTER_ACCESS_TOKEN_KEY | ||
| -# HUBOT_TWITTER_ACCESS_TOKEN_SECRET | ||
| +# HUBOT_TWITTER_ACCESS_TOKEN_KEY_<USERNAME> |
I think we can remove the _ from this one. In this particular script, we wouldn't support multiple accounts.
I think we can remove the _ from this one. In this particular script, we wouldn't support multiple accounts.
My thinking was that HUBOT_TWITTER_ACCESS_TOKEN_KEY and HUBOT_TWITTER_ACCESS_TOKEN_SECRET are deprecated - no need in having two environment variables for the same value, other than to explicitly specify the default one (though it really doesn't matter which account it uses).
My thinking was that HUBOT_TWITTER_ACCESS_TOKEN_KEY and HUBOT_TWITTER_ACCESS_TOKEN_SECRET are deprecated - no need in having two environment variables for the same value, other than to explicitly specify the default one (though it really doesn't matter which account it uses).
|
Updated to gracefully degrade when environment variables aren't set. |
We are actually moving away from adding scripts to repository in favor of separate npm packages per scripts. See #1113 for details, and let us know if you have any questions about getting going with that!
|
We are actually moving away from adding scripts to repository in favor of separate npm packages per scripts. See #1113 for details, and let us know if you have any questions about getting going with that! |
afeld commentedOct 10, 2013
This also changes (and normalizes) the way that the Twitter keys/secrets are specified, which is a breaking change.
/cc @technicalpickles @jhubert