Improve insult script #570

Merged
merged 3 commits into from Nov 6, 2012

Conversation

Projects
None yet
2 participants
Contributor

bval commented Sep 15, 2012

Hubot will now choose a random insult from randominsults.net rather than
the short array of built-in insults he previously had.

Owner

technicalpickles commented Oct 20, 2012

I'd recommend using cheerio rather than soupselect and htmlparser, becasue it has a much more usable interface. iced-coffee-weather.coffee has an example of pulling things out of html using it.

Contributor

bval commented Oct 22, 2012

Word. Thanks for the feedback. There's 17-18 scripts in src/scripts already using soupselect/htmlparser versus 1 using cheerio. If the intent is to move the rest to cheerio I'm happy to resubmit this pull, but otherwise wanted to avoid introducing a new dependency to hubot users in the field.

Cheers,

Brandon

Owner

technicalpickles commented Oct 22, 2012

It's a new dependency for hubot in the field, either way. Mostly because you can't guess if insult users are already using cheerio or other dependency.

As a maintainer, I'd rather see cheerio, because it's a lot easier to grok what markup is being selected, and what is being done with it. Lines like this always make me wince:

quote[0].children[0].data

On Oct 22, 2012, at 12:47 PM, Brandon Valentine notifications@github.com wrote:

Word. Thanks for the feedback. There's 17-18 scripts in src/scripts already using soupselect/htmlparser versus 1 using cheerio. If the intent is to move the rest to cheerio I'm happy to resubmit this pull, but otherwise wanted to avoid introducing a new dependency to hubot users in the field.

Cheers,

Brandon


Reply to this email directly or view it on GitHub.

bval added some commits Sep 15, 2012

@bval bval Improve insult script
Hubot will now choose a random insult from randominsults.net rather than
the short array of built-in insults he previously had.
1d90491
@bval bval Refactor improved insult script to use cheerio
Rip out soupselect/htmlparses in favor of cheerio
47fd15b
Contributor

bval commented Oct 23, 2012

I've added an additional commit to the pull that replaces soupselect/htmlparser with cheerio. Much cleaner. I had just initially gone with the tools in use already in the codebase. Now I'm tempted to go tacke the rest of them using soupselect/htmlparser.

Owner

technicalpickles commented Oct 23, 2012

Sure, go for it :) Just send a new pull request for them.

On Oct 22, 2012, at 9:38 PM, Brandon Valentine notifications@github.com wrote:

I've added an additional commit to the pull that replaces soupselect/htmlparser with cheerio. Much cleaner. I had just initially gone with the tools in use already in the codebase. Now I'm tempted to go tacke the rest of them using soupselect/htmlparser.


Reply to this email directly or view it on GitHub.

Owner

technicalpickles commented Oct 27, 2012

Don't forget to update the dependency to reflect cheerio :)

Contributor

bval commented Oct 27, 2012

Oh, right. That. :)

@technicalpickles technicalpickles added a commit that referenced this pull request Nov 6, 2012

@technicalpickles technicalpickles Merge pull request #570 from brandonvalentine/master
Improve insult script
5a2c07e

@technicalpickles technicalpickles merged commit 5a2c07e into github:master Nov 6, 2012

@amaltson amaltson pushed a commit to amaltson/hubot-scripts that referenced this pull request Jan 7, 2013

@technicalpickles technicalpickles Merge pull request #570 from brandonvalentine/master
Improve insult script
c70f13b
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment