Update rubocop to v0.52.1 for Slack plugin

[nicoleheejin]

Pull Request Checklist

Is this in reference to an existing issue?
Yeah! sensu-plugins/community#77

General

• Update Changelog following the conventions laid out here

• Update README with any necessary configuration snippets

• Binstubs are created if needed

  • This failed for me, will look into further after further progress on [CVE-2017-8418] Update ruby plugins for vulnerable rubocop dependency community#77.

• RuboCop passes

• Existing tests pass

Purpose

• There's a CVE out for Rubocop, and this fixes that. [CVE-2017-8418] Update ruby plugins for vulnerable rubocop dependency community#77

Known Compatibility Issues

• None that I'm aware of

[majormoses]

Thanks for your contribution to Sensu plugins! Without people like you submitting PRs we couldn't run the project. I will review it shortly.

Special kudos for picking up security issues.

[majormoses]

Overall this looks great! some minor things regarding our community contribution practices, once they are addressed I am 👍 Thank you very much for taking the time to do this.

[majormoses]

Per our changelog conventions all submitted changes go under ### [Unreleased] there are a couple of reasons for this:

• There is no guarantee when a maintainer will review so the release date will likely be wrong unless we get to it in the same day
• There is no guarantee on the review/merge order, bumping the version prior to acceptance is pretty much guaranteed to need to have the submitter rebase
• Maintainer may disagree on how you interpret the change, for example in this case you versioned a as a patch when in fact it is a breaking change as you drop a version of ruby being supported. Even when an update is in the name of security we generally follow semver unless there is a very good reason not to such as putting the consumer at extreme risk. This is outlined as it relates to security here if you are interested in understanding how we handle these edgecases.

[majormoses]

To keep things consistent with the other PR's we should bump it to ~> 0.51.0 which satisfies the requirement to patch the vulnerability and reduces the number of new cops to satisfy.

[majormoses]

This should actually go under a ### Security header rather than changed as it makes it clear to consumers that this update is important and should be prioritized over pulling in other types of changes.

[majormoses]

see above comment regarding versioning pre-acceptance.

[majormoses]

This needs to be called out in the changelog under ### Breaking Changes so that we inform users on the impact and make it more obvious to reviewers and maintainers so it can be evaluated and versioned appropriately.

[nicoleheejin]

awesome! I will address these changes shortly and will continue working through the list! Thank you for the feedback!

[majormoses]

@nicoleheejin checking back to see if you can make those changes so we can get this merged and released.

Also looks like you now need a rebase to fix conflicts, let me know if you need help.

[majormoses]

closing due to inactivity and merge conflicts.