hash-gaming / robotk

Mostly helpful but sometimes trolly
MIT License
2 stars 1 forks source link

Some kickass improvements #23

Closed paroxp closed 6 years ago

paroxp commented 6 years ago

What

In order to work with it, I need some clean shit going on. We intend to replace some of the hard-coded variables with configuration options. That's great. In order to prepare for that, let's separate this out from the code.

Let's enjoy what we're doing... Frankly the current state is disgusting! :troll:

I've decided to go with BDD as all of us are familiar with this either from ruby or go. mocha and chi are pretty good for that and are supported by already existing hubot-test-helper.

I've have let the .eslintrc.json file know, that we're using mocha, so we we can keep on going with our chosen standards...

Currently a user has to provide robotk with a link to a message that they'd like to lit.

We could make it a little smarter and also allow people to share some content with the phrase @robotk lit.

Currently robotk will come back with a single response at all times. This could get boring and makes things more bureaucratic than ordinary users would like to.

We're implementing a specific functionality to register a list of responses and then return a .random() one when requested.

We'd like to go forward with the confidence of application keep on working. Ideal situation would be to hold a set of tests, covering each line of code in our system. This however proves to be difficult for different reasons.

For instance, I'd like to test that lit command when successful would create a message in a separate channel. Sounds simple, yet the hubot testing helper does not support the functionality of multiple rooms. This issue has already been raised with upstream and needs yet to be resolved.

In order to keep hubot happy, I had to separate the script tests into a separate directory. In fact, two of them, as I've separated the response messages from script file, to be DRY about it when using in tests. The .messages.js file is obsolete and should be removed once we come out with ideal configuration system.

I took the lousy approach of implementing tests for the other functions. I'm sorry.

How to review

Closes #22

Extra notes

Can we totally not do this:

if (false) {
  //
}
else {
  //
}

Totally grinds my gears...