Rocketchat plugin #53

dhollinger · 2018-05-01T16:39:15Z

No description provided.

bastelfreak · 2018-05-01T16:42:30Z

lib/plugins/chatops.rb

@@ -21,6 +21,14 @@ def notify(message)
 message,
 http_options: @args[:http_options] || {},
 icon_emoji: @args[:icon_emoji]).notify
+ when 'rocketchat'
+ LOGGER.info("Sending Rocket.Chat webhook message to #{@url}")
+ Chatops::Rocketchat.new(@channel,


Personally I prefer it when all params in new() are in the next line, limits the indentation to one level

I believe that is against the Ruby Style Guide and I don't think we turned off those cops in Rubocop. I saw failures related to trying to put parameters in-between open/closing parentheses on their own lines. It wants the first and last parameters on the same line as the open/close.

Same goes when calling a class method after the .new(), it wants the final parameter, closing parenthesis, and method call all on the same line.

I'd rather stick to the style guide on this one

I found a solution that should satisfy you, @bastelfreak. I'm pretty sure it's close to what you are asking.

coveralls · 2018-05-01T16:58:43Z

Coverage increased (+0.6%) to 51.532% when pulling e2f28f6 on dhollinger:rocketchat_plugin into c5ebd2e on voxpupuli:master.

rnelson0 · 2018-05-01T18:18:46Z

README.md

@@ -104,7 +104,8 @@ Then configure the webhook to add your Slack Webhook URL:
 ``` yaml
 chatops: true
 chatops_service: 'slack' # Required so the app knows that you're sending to Slack.
-chatops_channel: '#channel' # deftaults to #general
+chatops_url: 'http://slack.webhook/webhook' # mandatory for usage


example.com is safe for samples.

True, but slack is always https://hooks.slack.com/<hash>/<hash>/<hash>.

I'll update it with that at least.

rnelson0 · 2018-05-01T18:19:15Z

README.md

+##### Rocket.Chat Configuration
+
+You can enable Rocket.Chat notifications for the webhook. You will need a
+Rocket.Chat incoming webhook URL and the `rocket-chat-notifier` gem installed.


Aside: Will the runtime dependency provide this during installation, or do we need to plan to provide that another way? And if the runtime dep does, should it for an optional component?

As a runtime dependency, the rocket-chat-notifier gem will be installed as a dependency. We could make the gem "optional", but the more you do that, the more work you put on your users to install those "optional" dependencies themselves.

Think of puppetlabs-beaker dependencies, there are so many dependencies for each supported hypervisor that you either install a ton of deps during gem install or put the entire onus on the user to install what he needs (though he may not know what he needs).

I think at this point, we should leave it as a runtime dependency and instead make it optional in the future by providing the ChatOps plugin as a separate gem for puppet_webhook unto itself.

@rnelson0 I should probably add that, right now, the Chatops class doesn't test to see if a dependency exists before loading it, so it expects all dependencies to be there.

Just wanted to understand what the model looks like. Something to keep in mind later if we see dependencies explode.

rnelson0 · 2018-05-01T18:19:58Z

lib/plugins/chatops.rb

- http_options: @args[:http_options] || {},
- icon_emoji: @args[:icon_emoji]).notify
+ Chatops::Slack.new(
+ @channel, @url, @user, message, http_options: @args[:http_options] || {}, icon_emoji: @args[:icon_emoji]


I do feel the vertical stacking esp with pipes and braces is a little easier to digest, here and immediately below, but certainly not required.

I'll let you and @bastelfreak fight over this one 😃

oh sorry I wasn't clear. I meant something like this:

Chatops::Slack.new( bla, bla2, )

that's also enforced via rubocop in our modules, but depending on the current code it doesn't detect it / accepts different styles as well.

ahh, ok. I fixed that

binford2k · 2018-05-01T18:53:42Z

lib/plugins/chatops.rb

- message,
- http_options: @args[:http_options] || {},
- icon_emoji: @args[:icon_emoji]).notify
+ Chatops::Slack.new(


You might consider moving the require 'plugins/chatops/slack' in here. That will only load the integration you want to use. Not a big deal now, but as you start adding more integrations, you might not want them all loaded all the time.

binford2k · 2018-05-01T18:54:02Z

lib/plugins/chatops.rb

+ Chatops::Slack.new(
+ @channel, @url, @user, message, http_options: @args[:http_options] || {}, icon_emoji: @args[:icon_emoji]
+ ).notify
+ when 'rocketchat'


Don't forget to require 'plugins/chatops/rocketchat'

I knew I forgot something

The requested changes were made, but in a different location than the comment.

dhollinger added in progress enhancement needs-docs and removed in progress labels May 1, 2018

bastelfreak reviewed May 1, 2018

View reviewed changes

voxpupuli deleted a comment May 1, 2018

dhollinger force-pushed the rocketchat_plugin branch 2 times, most recently from dbc524b to c1c363b Compare May 1, 2018 16:49

voxpupuli deleted a comment May 1, 2018

dhollinger force-pushed the rocketchat_plugin branch 3 times, most recently from 01b5924 to b6e74be Compare May 1, 2018 16:50

David Hollinger added 2 commits May 1, 2018 11:51

Add rocket-chat-notifier runtime dependency

8c7195b

Add Rocket.Chat support to ChatOps plugin

e53378c

dhollinger force-pushed the rocketchat_plugin branch from b6e74be to 9b17cc8 Compare May 1, 2018 16:51

voxpupuli deleted a comment May 1, 2018

dhollinger removed the needs-docs label May 1, 2018

rnelson0 reviewed May 1, 2018

View reviewed changes

binford2k previously requested changes May 1, 2018

View reviewed changes

voxpupuli deleted a comment May 1, 2018

dhollinger force-pushed the rocketchat_plugin branch from c8c9111 to b5a36ac Compare May 1, 2018 19:04

David Hollinger added 3 commits May 1, 2018 14:05

Add Rocket.Chat documentation

d6c44ac

Rubocop fixes

b0066bc

Only load chatops subclasses when needed

e95cff8

dhollinger force-pushed the rocketchat_plugin branch from b5a36ac to e95cff8 Compare May 1, 2018 19:05

Update formate within notify case statement

e2f28f6

voxpupuli deleted a comment May 2, 2018

rnelson0 approved these changes May 2, 2018

View reviewed changes

binford2k approved these changes May 2, 2018

View reviewed changes

dhollinger merged commit 0515a4e into voxpupuli:master May 2, 2018

dhollinger deleted the rocketchat_plugin branch May 2, 2018 18:50

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rocketchat plugin #53

Rocketchat plugin #53

dhollinger commented May 1, 2018

bastelfreak May 1, 2018

dhollinger May 1, 2018 •

edited

Loading

dhollinger May 1, 2018

coveralls commented May 1, 2018 •

edited

Loading

rnelson0 May 1, 2018

dhollinger May 1, 2018 •

edited

Loading

rnelson0 May 1, 2018

dhollinger May 1, 2018

dhollinger May 1, 2018

rnelson0 May 2, 2018

rnelson0 May 1, 2018

dhollinger May 1, 2018

bastelfreak May 1, 2018

dhollinger May 2, 2018

binford2k May 1, 2018

binford2k May 1, 2018

dhollinger May 1, 2018

Rocketchat plugin #53

Rocketchat plugin #53

Conversation

dhollinger commented May 1, 2018

Choose a reason for hiding this comment

dhollinger May 1, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coveralls commented May 1, 2018 • edited Loading

Choose a reason for hiding this comment

dhollinger May 1, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dhollinger May 1, 2018 •

edited

Loading

coveralls commented May 1, 2018 •

edited

Loading

dhollinger May 1, 2018 •

edited

Loading