Added gif support for 9gag. #1304

Merged
merged 3 commits into from Feb 10, 2014

Projects

None yet

2 participants

@dedeibel
dedeibel commented Feb 4, 2014

In the default version of the script only the jpg version of gifs were shown which are usually pretty boring.

@technicalpickles technicalpickles and 1 other commented on an outdated diff Feb 4, 2014
src/scripts/9gag.coffee
@@ -6,13 +6,16 @@
# "soupselect": "0.2.0"
#
# Configuration:
-# None
+# NINEGAG_NO_GIFS (optional; default is "false")
@technicalpickles
technicalpickles Feb 4, 2014 GitHub member

Can you prefix this environment variable with HUBOT_? That'd also let call it HUBOT_9GAG_NO_GIFS.

@dedeibel
dedeibel Feb 4, 2014

Am 02/04/14 16:42, schrieb Josh Nichols:

Can you prefix this environment variable with HUBOT_? That'd also let
call it HUBOT_9GAG_NO_GIFS.

yes, sure. done.

@technicalpickles technicalpickles and 1 other commented on an outdated diff Feb 4, 2014
src/scripts/9gag.coffee
@@ -37,16 +40,24 @@ send_meme = (message, location, response_handler)->
location = response.headers['location']
return send_meme( message, location, response_handler )
- img_src = get_meme_image( body, ".badge-item-img" )
+ no_gifs = process.env.NINEGAG_NO_GIFS
@technicalpickles
technicalpickles Feb 4, 2014 GitHub member

You could use the existential operator here to make no_gifs a boolean, ie:

no_gifs = process.env.NINEGAG_NO_GIFS?

That'd simplify the conditional below.

@dedeibel
dedeibel Feb 4, 2014

Am 02/04/14 16:44, schrieb Josh Nichols:

You could use the existential operator here to make no_gifs a boolean, ie:

no_gifs = process.env.NINEGAG_NO_GIFS?

That'd simplify the conditional below.

Didn't know about it yet. Good idea. Changed it.

@dedeibel
dedeibel commented Feb 5, 2014

Changed the gifs config variable name and the semantic to "defined" or not.

@dedeibel dedeibel closed this Feb 6, 2014
@dedeibel dedeibel reopened this Feb 6, 2014
@dedeibel
dedeibel commented Feb 6, 2014

(sorry, wasn't so sure what the close button does)

@dedeibel
dedeibel commented Feb 6, 2014

In my other branch I also included the title feature from pull request #1276 - are you interested?
dedeibel@7c91c3a

@technicalpickles technicalpickles merged commit 068ec13 into github:master Feb 10, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment