shockey / do Goto Github PK

We have plenty of testable logic in the app, and an easily mockable slackbot instance.

Let's write some tests! 😈

Examples

• @do pm2 start a: starts the a pm2 process
• @do pm2 stop b: stops the b pm2 process
• @do pm2 restart c: restarts the c pm2 process
• @do pm2 list: List current processes

Currently, the codebase is a bit murky in regards to what data is being sent where. Problems include inconsistent variable naming, and a relative lack of comments.

Currently, anyone on a Slack team can invoke any command. This is suboptimal from a security perspective: consider a use case where Do is running on a production system.

Possible solution: a user is defined as the superuser (by handle or some other unique Slack identifier) in Do config, and commands are added to instruct Do to start and stop trusting additional users. Those users, in turn, can instruct Do to start or stop trusting any other users (except the user defined as superuser in the config).

It'd be awesome to be able to update do via Slack.

Do could, itself, be a target, with an update verb that runs git pull and then exits, allowing the process manager running Do to restart it.

Slack puts a colon after auto-completed names; we should tolerate that.

>>> pm2 restart -s wmsv-wmffmpeg 
/home/ubuntu/.nvm/versions/node/v4.2.6/lib/node_modules/pm2/node_modules/pm2-axon/node_modules/amp-message/index.js:126
  return new Buffer('j:' + JSON.stringify(arg));
                                ^
TypeError: Converting circular structure to JSON
    at Object.stringify (native)
    at pack (/home/ubuntu/.nvm/versions/node/v4.2.6/lib/node_modules/pm2/node_modules/pm2-axon/node_modules/amp-message/index.js:126:33)
    at encode (/home/ubuntu/.nvm/versions/node/v4.2.6/lib/node_modules/pm2/node_modules/pm2-axon/node_modules/amp-message/index.js:101:14)
    at Message.toBuffer (/home/ubuntu/.nvm/versions/node/v4.2.6/lib/node_modules/pm2/node_modules/pm2-axon/node_modules/amp-message/index.js:68:10)
    at ReqSocket.Socket.pack (/home/ubuntu/.nvm/versions/node/v4.2.6/lib/node_modules/pm2/node_modules/pm2-axon/lib/sockets/sock.js:92:14)
    at ReqSocket.send (/home/ubuntu/.nvm/versions/node/v4.2.6/lib/node_modules/pm2/node_modules/pm2-axon/lib/sockets/req.js:96:21)
    at Client.call (/home/ubuntu/.nvm/versions/node/v4.2.6/lib/node_modules/pm2/node_modules/pm2-axon-rpc/lib/client.js:34:13)
    at Object.executeRemote (/home/ubuntu/.nvm/versions/node/v4.2.6/lib/node_modules/pm2/lib/Satan.js:563:23)
    at /home/ubuntu/.nvm/versions/node/v4.2.6/lib/node_modules/pm2/lib/CLI.js:1047:13
    at /home/ubuntu/.nvm/versions/node/v4.2.6/lib/node_modules/pm2/node_modules/async/lib/async.js:181:20

I think there may be some isolation regarding our runner process system that’s keeping the pm2 binary from communicating with its god process.

We have an empty wizard.js file in the repository.

There should be a config wizard there 😄

• Adopt sample PM2 file
• walk user through obtaining a Slack token
• set output channel and emoji

The sequence API is kinda weird, in that you usually end up passing an array of words broken by spaces, for example ['npm', 'install'].

I've yet to see a use case that breaks that pattern, actually.

We could save users some keystrokes (and headaches) by allowing them to pass in a string (for example, npm install, which will be broken down into the command and array of arguments to be passed into exec behind the scenes.

Beacuse of the way the verb runner is written (that is, synchronously), Do is unable to handle any incoming requests while it's executing a verb sequence.

This may cause problems with Slack connectivity and state when running very long verb sequences.

We should refactor the verb runner to an asynchronous shell execution API, being careful to preserve the waterfall pattern of a sequence's execution.

We should adopt the WeMash ESLint presets.

• be more focused on end user
• add bit about inviting bot to a room
• need more clarity on the target api, sequences...
• screenshots would be nice
• developer section: overview of data flow, what reused variable names conceptually represent, etc

Currently, we have to prefix Do commands with @do even when messaging Do directly.

This should keep us from having any issues regarding changes to data in bundles for incoming messages, etc

note: need to add clarity to this issue

If I forget to add a required key to a target, I get this:

Oops! There was a problem parsing targets/example.js: 
'AssertionError: expected { Object (verbs, description, ...) } to have keys 'verbs', 'name', 'description', and 'workingDir''

While this is helpful, it would be nice to be more user-friendly.

Let's start using the PM2 API to start Do in the background, and package a local PM2 with Do so it works without needing to install PM2 globally.

Currently, handlers for help, hello, and list are hardcoded into index.js.

We should break those out into a module system similar to targets, so they're easier to reason about and easier to create and customize.

This is a bug newly introduced by the resolution of #1.

Running child processes asynchronously allows us to kick off the same verb twice at once, which is a bad thing: being at two places in a build pipeline at once can cause some seriously subtle headaches to end users.

We should block a verb from starting if it's already running!

Imagine we wanted a log verb that outputs the last n lines from a process manager.

Possible formats, where n = 100:

• @do log example 100
• @do log example n:100

Accepting arguments may require us to augment the sequence API (which is going on in #3 for 1.0 as well).

We'll also want to sanitize the argument inputs- though we can generally trust users, we can't Trust Users™, so let's stay on the safe side.

Possible solutions:

• assert that filename matches the name property of the target
• remove the name target property and instead use the file's name

Currently, due to ES6 presence in the codebase, Do requires node >=6.0.0.

We can transpile ES6 on the fly in older Node versions using Babel's require hook. This may have runtime performance implications, but IMO it's better to run more slowly in older Nodes than to not run at all 😃