Update Rubocop to v0.52.1 for Zendesk plugin #2
Conversation
|
Thank you for submitting this, I will review shortly. |
| @@ -4,6 +4,11 @@ This project adheres to [Semantic Versioning](http://semver.org/). | |||
| This CHANGELOG follows the format listed at [Keep A Changelog](http://keepachangelog.com/) | |||
|
|
|||
| ## [Unreleased] | |||
|
|
|||
| ## [1.1.0] - 2018-01-21 | |||
There was a problem hiding this comment.
As mentioned in the other PR, 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.
| @@ -4,6 +4,11 @@ This project adheres to [Semantic Versioning](http://semver.org/). | |||
| This CHANGELOG follows the format listed at [Keep A Changelog](http://keepachangelog.com/) | |||
|
|
|||
| ## [Unreleased] | |||
|
|
|||
| ## [1.1.0] - 2018-01-21 | |||
| ### Changed | |||
There was a problem hiding this comment.
As mentioned in the other PR, 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.
|
|
||
| ## [1.1.0] - 2018-01-21 | ||
| ### Changed | ||
| Bumped Rubocop to v0.52.1 for CVE 2017-8418 (@nicoleheejin) |
There was a problem hiding this comment.
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.
| @@ -21,23 +21,23 @@ Gem::Specification.new do |s| | |||
| s.platform = Gem::Platform::RUBY | |||
| s.post_install_message = 'You can use the embedded Ruby by setting EMBEDDED_RUBY=true in /etc/default/sensu' | |||
| s.require_paths = ['lib'] | |||
| s.required_ruby_version = '>= 2.0.0' | |||
| s.required_ruby_version = '>= 2.1.0' | |||
There was a problem hiding this comment.
As mentioned in the other PR, 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.
| s.add_development_dependency 'rspec', '~> 3.4' | ||
| s.add_development_dependency 'rubocop', '~> 0.52.1' |
There was a problem hiding this comment.
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.
|
@nicoleheejin checking back to see if you can make those changes so we can get this merged and released. |
|
Hey @nicoleheejin! We're going to move forward with #3 as a fix over this PR. It's our bad that it wasn't clear which version of rubocop to address - hopefully that's clearer now with updates to sensu-plugins/community#77. And we totally understand people get busy, so I hope you'll keep contributing when you have the time again. Thank you so much and tons of #monitoringlove. Closing in favor of #3. |
Pull Request Checklist
Is this in reference to an existing issue?
Yeah! sensu-plugins/community#77
General
Update Changelog following the conventions laid out on Keep A Changelog
Update README with any necessary configuration snippets
Binstubs are created if needed
Fails for me due to unrelated code, will take a look afterward
RuboCop passes
Existing tests pass
Purpose
There's a CVE out for Rubocop, and this fixes it. sensu-plugins/community#77
Known Compatability Issues
None to my knowledge.