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

Twitter commands #6

Merged
merged 24 commits into from
Jan 23, 2016
Merged

Twitter commands #6

merged 24 commits into from
Jan 23, 2016

Conversation

citypaul
Copy link
Contributor

So I made a few changes here to make the twitter stuff a bit nicer.

I should say I'm doing this blind without a robot and without... tests! Shock horror! So I can't confirm it works unless you try it out for me.

I refactored some of the commands so they no longer create the bb8 instance. I figured they are just commands and should do nothing more than take an instance of bb8 that is already created and run commands against him. This meant I could then pass commands from twitter quite easily. I had a go at refactoring the index script accordingly.

There's a config folder with a place for you to store your twitter api details. I couldn't see a way of using twitter's api without these details. If there is a way please alter the code or let me know and I'll do it...

The twitter command is pretty basic at the moment - it polls twitter every 10 seconds searching for the hashtag #bb8bbc, and then looks for the first hashtag it finds in that tweet. It then simply issues that as the command.

It doesn't check to see whether the command exists or deal with more than one command etc right now, but it's a start.

So to make BB8 roll you'd do:

#roll #bb8bbc

I think extracting the commands as functions that simply receive a bb8 instance is a nice improvement - removes duplication and means you could start combining the commands together in theory.

Dunno if it works though! Try it out :)

@@ -0,0 +1,6 @@
module.exports = {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Obviously to get this to work you need to add your twitter api details here, and remove the .EXAMPLE from this file. If there's a better way of doing this without creating this stuff just let me know and I'll change it :-)

bb8.randomColor();
setInterval(function () {
bb8.randomColor();
}, 1000);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This feels a bit more betterer ;)

Copy link
Owner

Choose a reason for hiding this comment

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

👍

require(command)(bb8)
});
}
};
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This block from line 3-11 could probably be extracted into its own lib...

@@ -0,0 +1,6 @@
module.exports = {
consumer_key: '',
consumer_secret: '',
Copy link
Owner

Choose a reason for hiding this comment

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

  1. do you need the consumer key and secret? back in the day you could get away with just the access_token_key and access_token_secret similar to what I wrote in the Github wrapper in libs?
  2. Might be good to do it like that to be consistent. I made it so it reads either a config passed in through the CLI or a ENV variable (process.env.NAME_OF_VAR)

@mintuz
Copy link
Owner

mintuz commented Jan 20, 2016

@citypaul Few minor things but looks good otherwise 👍 Add yourself to the README as well ;)


setInterval(function () {
executeTwitterCommand();
}, 10000);
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'll make this delay param configurable too...

Copy link
Owner

Choose a reason for hiding this comment

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

👍

@citypaul
Copy link
Contributor Author

Ok, updated a few things this morning:

  • Can now set the hashtag to search for from the command line
  • Can now set the interval time for searching twitter from command line
  • Twitter api config can come through from environment variables or cli options

I tried querying twitter without the consumer key and consumer secret, but it didn't work, so for now I've left this in. If there is a way of removing these, please go ahead and remove them 👍

@mintuz
Copy link
Owner

mintuz commented Jan 23, 2016

@citypaul I've done some tweaks to your code. It's working nicely now. Swapped some code for some lodash.

@citypaul
Copy link
Contributor Author

@mintuz Nice! I hadn't thought about the fact the robot would keep getting the same command issued... One of the perils of not having an actual robot (and not writing tests I guess 😉).

Looks good though. I need to read up on the lodash stuff. That _.partial method looks interesting...

@mintuz
Copy link
Owner

mintuz commented Jan 23, 2016

_.partial can partially apply some of the arguments to a function and return a new function with the parematers that have not been passed in. I'm only using it because when it comes to testing it (eventually) it's easier to do dependency injection. I also think it makes the code look slightly nicer. You don't have to use it in this circumstance I just thought it would be nice.

@mintuz
Copy link
Owner

mintuz commented Jan 23, 2016

Anyways going to merge. Good work 👍 now go buy one 😄

mintuz added a commit that referenced this pull request Jan 23, 2016
@mintuz mintuz merged commit 82ed900 into mintuz:master Jan 23, 2016
@mintuz mintuz deleted the twitter branch January 23, 2016 20:28
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