One Script To Rule Them All (or at least the really simple ones) #112
Conversation
|
|
||
| module.exports = (robot) -> | ||
| commands = _.keys(commandMap) | ||
| for command in commands |
There was a problem hiding this comment.
Why not chain a _.map here instead of a for?
There was a problem hiding this comment.
i feel like map is good for getting an object back out after iterating over the list vs. just iterating over a list and calling a method.
There was a problem hiding this comment.
Ah, good point. Didn't realize that was a void return.
|
Oh and I do have the list of commands ready to load up hipchat with the old phrases |
| parseInt(percentString, 10) | ||
|
|
||
| tellRobotWhatToDo = (robot, command, key) -> | ||
| robot[commandMap[command]](new RegExp("(#{key}(\\s|$))", 'i'), (msg) -> |
There was a problem hiding this comment.
Is there a possibility of someone doing something nasty here? (think injection)
There was a problem hiding this comment.
thats an interesting thought, I guess I could try something fun out.
There was a problem hiding this comment.
so I was thinking if you could close of the " in the regex early than you probably could inject something, however the input regex would capture the ". So I tried sending in the ascii char and a unicode char, but hipchat sends those as just plain characters. I don't think you can close off the " in the regexp constructor, but I could also be dead wrong.
|
The only thing that worries me is that if redis dies somehow, we'll lose all of these. It might be useful to look into redis backup. |
|
alternatively we could also use a hipchat bot called Hearsay to do all these things too: |
|
Part of me will miss all of these small scripts, even though they all technically repeat code. I think it's nice to see a code history of scripts that are added and remember what was happening at Labs at the time to inspire those scripts (i.e. qfax). Merging this script may have a negative effect on that history as well as general contribution to Hubot. Maybe I'm overthinking it; what do you think? |
|
I had the same thoughts too. (I know Jenai said she wanted to learn to write a script to do one of the small things, and I don't want to take away that either.) I was only thinking there are common complaints like the frequency of responses and also trying to make sure we live clean code in everything we do. |
|
@maschall I definitely see the merits in both things you said. I think we just need to integrate this script into hubot, see how we all like it, and assess from there! |
|
ok, maybe I should un remove the old scripts? |
|
Perhaps we use the new system for a bit and then revert 59ca09c if people want individual scripts back? |
This reverts commit 59ca09c.
One Script To Rule Them All (or at least the really simple ones)
I wanted to test my coffeescript skills, and thought this might be better way of doing all the simple scripts.
I'm going with the simple approach, if we want to remove a key, than we set the percent to 0%.
What ya think?
If this is ok, I will expand this to be able to take multiple responses with a random.
Also this will accept regex for the key so "(damn it|damnit|dammit|damnit) jeff (kelley|kelly)" can map to I don't get it