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.

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.

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.

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

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.

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

I knew I forgot something