bocoup / bots

Our bots.
8 stars 8 forks source link

Expertise alert fixes #108

Closed tkellen closed 8 years ago

tkellen commented 8 years ago

/cc @cowboy

I will be landing this if you can't review by EOD Friday.

cowboy commented 8 years ago

I just added a small commit to show a more meaningful error message in cases where a scheduler cron pattern is invalid.

cowboy commented 8 years ago

Other than my comment, :+1:

The throttler is really interesting. My only comment is that I wish the promise library option was named differently, so one could do this:

const throttle = new Throttle({
  requestsPerSecond: 1,
  Promise,
});
cowboy commented 8 years ago

Actually, I'm :-1: until my 2nd comment is answered. What's currently there is confusing, maybe broken.

tkellen commented 8 years ago

Updated based on your suggestions, with a few improvements.

cowboy commented 8 years ago

Nice. Go team! :shipit:

tkellen commented 8 years ago

:D