Skip to content
This repository has been archived by the owner on Jun 8, 2023. It is now read-only.

Added support for multiple actions #1604

Merged
merged 2 commits into from
Jun 5, 2015

Conversation

inhumantsar
Copy link
Contributor

We wanted to be able to trigger multiple events in a sequence with eardropping but the existing implementation would trigger events in an almost random order. Added an order parameter to the task object and set up a bit to sort tasks that match the current eardrop trigger. I'm quite new to coffeescript and hubot programming so there's probably some room to refactor my additions.

The ordering still isn't 100% accurate. Hubot will trigger the tasks in the correct sequence but target actions may appear to the user out of order. For example, when hubot hears "kaboom" it should trigger "how many days since kaboom?", then echo and finally reset the days since kaboom counter to 0. On the server side, this executes in the desired order but on client side, those messages might be received out of sequence.

We wanted to be able to trigger multiple events in a sequence with eardropping but the existing implementation would trigger events in an almost random order. Added an order parameter to the task object and set up a bit to sort tasks that match the current eardrop trigger. I'm quite new to coffeescript and hubot programming so there's probably some room to refactor my additions.

The ordering still isn't 100% accurate. Hubot will trigger the tasks in the correct sequence but target actions may appear to the user out of order. For example, when hubot hears "kaboom" it should trigger "how many days since kaboom?", then echo <url to a gif> and finally reset the days since kaboom counter to 0. On the server side, this executes in the desired order but on client side, those messages might be received out of sequence.
tasks.sort (a,b) ->
return if a.order >= b.order then 1 else -1

console.log "Tasks: #{tasks}"
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks like a debugging statement to remove. At the very least, I'd use robot.logger.debug instead so HUBOT_LOG_LEVEL is respected.

@inhumantsar
Copy link
Contributor Author

Took out the debug line, it wasn't a terribly useful one anyway.

@technicalpickles
Copy link
Contributor

We are actually moving away from adding scripts to repository in favor of separate npm packages per scripts. We have already stopped accepting new scripts, and will stop accepting pull requests on this repository after hubot 3.0.

See #1113 for details. If you are interested in maintaining this longer term, check npm in case someone already made a package for it, and if not, check out https://hubot.github.com/docs/scripting/ for creating a package of your own.

technicalpickles added a commit that referenced this pull request Jun 5, 2015
Added support for multiple actions
@technicalpickles technicalpickles merged commit bcc20ba into github:master Jun 5, 2015
@inhumantsar
Copy link
Contributor Author

Not sure if I put this PR in before or after the repo breakup but I'm aware and will be working on the separate repos should the need arise. Thanks for the merge!

@technicalpickles
Copy link
Contributor

@inhumantsar no problem! If you do split out a pacakge, please drop an update on #1641 with a link to the new home.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants