Skip to content
This repository has been archived by the owner on Feb 15, 2022. It is now read-only.

Add notifiers: slack, ifttt, xmpp, pushbullet #595

Merged
merged 1 commit into from
Oct 6, 2017
Merged

Add notifiers: slack, ifttt, xmpp, pushbullet #595

merged 1 commit into from
Oct 6, 2017

Conversation

DeviaVir
Copy link
Owner

@DeviaVir DeviaVir commented Oct 6, 2017

The work from @emabo and @flocca unified, but as extensions.

Please review:

Copy link
Contributor

@evseevnn-zz evseevnn-zz left a 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

module.exports = function container (get, set, clear) {
var ifttt = {
pushMessage: function(config, title, message) {
var request = require('request')
Copy link
Contributor

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.

Copy link
Owner Author

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.

console.log('error: Push message failed, ' + err)
return
}
// console.log('info: Push message result, ' + res)
Copy link
Contributor

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

@DeviaVir
Copy link
Owner Author

DeviaVir commented Oct 6, 2017

@evseevnn good stuff, added.

var postData = {'value1': title , 'value2': message }

function callback(error, response, body) {
if (!error) {
Copy link
Contributor

@emabo emabo Oct 6, 2017

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.
Copy link
Contributor

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.
Copy link
Contributor

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

Copy link
Contributor

@emabo emabo left a 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

@DeviaVir DeviaVir merged commit af42c42 into DeviaVir:master Oct 6, 2017
@DeviaVir DeviaVir deleted the notifiers branch October 6, 2017 16:01
@flocca
Copy link

flocca commented Oct 6, 2017

Great job @DeviaVir, it works like a charm! 😉

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants