-
Notifications
You must be signed in to change notification settings - Fork 2k
Add notifiers: slack, ifttt, xmpp, pushbullet #595
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also need append documentations in README.md
extensions/notifiers/ifttt.js
Outdated
module.exports = function container (get, set, clear) { | ||
var ifttt = { | ||
pushMessage: function(config, title, message) { | ||
var request = require('request') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why you use the require inside the function? May be better do it before line 1, before module.exports
.
Just my thinks. I know what V8 will optimise this, but it bad tone.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I originally thought my IDE was telling me the require should be inside the function, vs outside of it, turned out I was just overwriting the var, leaving one unused (the actual error). I amended this now.
extensions/notifiers/pushbullet.js
Outdated
console.log('error: Push message failed, ' + err) | ||
return | ||
} | ||
// console.log('info: Push message result, ' + res) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please, if we not use some code better remove it and not clog up the code
@evseevnn good stuff, added. |
extensions/notifiers/ifttt.js
Outdated
var postData = {'value1': title , 'value2': message } | ||
|
||
function callback(error, response, body) { | ||
if (!error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I saw you already removed logs when no errors are reported in other notifiers, so I would do the same here, or at least, put it under c.debug.
|
||
### pushbullet | ||
|
||
Supply zenbot with your api key and device ID and we will send your notifications to your device. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if you want we can add a ref URL for the service: https://www.pushbullet.com/
README.md
Outdated
|
||
### IFTTT | ||
|
||
Supply zenbot with your IFTTT maker key and zenbot will push notifications to your IFTTT. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
here the service has been integrated using webhooks of IFTTT: https://ifttt.com/maker_webhooks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
made some comments in README and extensions/notifiers/ifttt.js
Great job @DeviaVir, it works like a charm! 😉 |
The work from @emabo and @flocca unified, but as extensions.
Please review: