review for awesomeness?

[gregglind]

@bwinton, What do you think about this?

GL

[bwinton]

Random nit-ish comments: 1) I prefer using "self" to "that", since it seems more like the same object as this… 2) I think it's easier to read: if (unwatch_list === undefined) unwatch_list = Object.keys(this._watched); than: unwatch_list === undefined ? unwatch_list = Object.keys(this._watched) : unwatch_list; 3) You really need the API docs to appear between the Example and the FAQ, so that the question "Why have a studyid?" makes sense to people.

I like the example, although I'm slightly unclear as to what data will send where. Hopefully the API docs will tell me that, although a sample add-on which used this code to log some data would be a good idea, too.

[gregglind]

1) Alas, "self" is a module in Jetpack terms. For now, I am going to leave these as is, to avoid confusion.

2) Done.

3) What do you suggest for "API docs"?

[bwinton]

For #3, the word "studyid" occurs only once in the readme, so "Why have a studyid?" is unlikely to be a frequently asked question (at least, not at that point in the user's interaction with this codebase ;).

For API docs, I'm thinking something like this:

require("micropilot")

returns:

• uu: function()

• UPLOAD_URL: String

  A description of what the UPLOAD_URL is used for.

• EventStore: Class
• Micropilot: Class
• snoop: function()
• Fuse: Class

• main: function(options, callback)

  (Maybe you don't want to document this, if it's not part of the official API. :)

uu

• returns a new UUID with the "{}"s stripped off.

EventStore

methods:

• initialize: function(collection, keyname)

  Some comments about what initialize does.

• type: String
• db: function()
• add: function(data)
• getAll: function()
• clear: function()

Micropilot

methods:

• initialize: function(studyid)

  This is the mention of studyid that will lead people to ask "Why have a studyid", or if they've skipped ahead to the faq, it will at least let them know where the studyid that was mentioned should be specified.

• type: String

  What the heck is type for, anyways?

• startdate: Date Property
• data: function() …

If you were extra cool, you could automatically extract this from the code itself, and put a link to the API docs in the readme. ;) Does that all make sense?

Later, Blake.

[bwinton]

I think this looks pretty good, although there are some outstanding questions/requests.

1. What is a topic? (I think it's just an event that the addon can send to log something, but it's not explained.)
2. Since the most common case will be to have:

  .then(function (mtp) {
    mtp.upload(UPLOAD_URL);
    mtp.stop()
  })

it would be nice to have that either be the default, or have a convenience method to do that.

1. Oh, actually, that reminds me, UPLOAD_URL should be on the monitor, so that people can type let monitor = require('micropilot').Micropilot('tabsmonitor');, and still have access to it for the upload. (Although, if you add the convenience method, this is less of a problem. :)
2. It would be good to be able to customize the watch function, so that I can pre-parse the data before it gets stored in the EventStore. (i.e. If I want to log the number of clicks on the "help" button, but don't want to have a record for each click, I could write something like:

  .preprocess(function (topic, data) {
    if (topic === 'button.click') {
      this.get('button.click', function (storedData) {
        storedData.count += data.count;
        this.record(storedData);
      });
    }
  })

(That's obviously not actually going to work for a number of reasons, but you get the idea, I hope. ;)

Thanks, Blake.

[gregglind]

It is now awesome.