Open majormoses opened 6 years ago
Prompted by this issue: https://github.com/sensu-plugins/community/issues/78 please leave your comments, thoughts, etc.
Picking up sensu-plugins-ansible
I started at the top so I am working on ansible, might be best for you to start at the bottom and work up.
@majormoses sounds like a plan 👍
Take a look at: https://github.com/sensu-plugins/sensu-plugins-ansible/pull/6 which is an example and why I created the security issue linked above (avoiding link spam) one to discuss how to version it.
Seeing this erroneous rubocop failure with Rubocop 0.51:
bin/metrics-zookeeper-cluster.rb:52:60: C: Do not place comments on the same line as the def keyword.
def follow_url(uri_str, agent = "sensu-plugins-zookeeper/#{SensuPluginsZookeeper::Version::VER_STRING}", max_attempts = 10, timeout = 10)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
There is a fix, but it has not been released yet: bbatsov/rubocop#4886
Until that is released, shall we disable the Style/CommentedKeyword
cop via .rubocop.yml ?
I'd prefer we disable it inline and link above it with a TODO:
to remove it later. More specific disabling is generally better than blanket disabling it. It leaves it to the developer to treat rubocop like the pirates code. A knowledgeable developer will be able to look at the code and say that cop is wrong and not blindly find it there but realize there are other times where it is pushing you to write better code.
Another is that Metrics/BlockLength
is enabled by default in a newer version of Rubocop. A caveat I find with this one is, for example, a gemspec file is likely to exceed the default maximum 25 lines if you have a good few dependancies included. Tend to find with this cop, you have to be fairly pragmatic and disable as appropriate (e.g. gemspecs!). I'm doing this inline also, but we should discuss this further. Thoughts?
I've raised my first PR for this (sensu-plugins/sensu-plugins-zookeeper#15). Would appreciate a review before I crack on with anymore please, make sure I'm on the right track :)
I thought these would be a really quick change.. but of course significant bump in rubocop version means many more additional cops and therefore further amendments required.
I have been disabling it inline for the gemspec as there is no value there, there is good reason to keep blocks relatively short as they are basically being passed to a function. I don't want to make a blanket wide decision yet on this without analyzing the impact. In other cases if it's not a trivial refactor I'd disable it inline with a TODO comment.
I thought these would be a really quick change.. but of course significant bump in rubocop version means many more additional cops and therefore further amendments required.
Story of my life, the aws one was pretty gnarly.
I think we probably want to maybe tweak the default to say enable it but limit it to 50 (opposed to the 25) lines in a block. Honestly if you need 50 lines you really need helper functions. @sensu-plugins/commit-bit please weigh in.
Planning on taking a look at this now! Believe I got it to pass, but please let me know if there's anything I should add. Thanks!
Picking up sensu-plugins-chef
.
If no one claims these first in a couple sprints my team will knock out any of the ones we use and are not already done:
@majormoses fyi, I updated the initial issue with the exact version requested as of our community chat (and your blog post)
UPDATE (2/6/18) - Please specifically version to
~> 0.51.0
for consistency across plugins.Original post:
Update rubocop gems to 0.51+ to mitigate issue: https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2017-8418 This is a very low severity warning (and agree after reading the impact) but it is a security one and could be a relatively easy to divide and conquer. If anyone would like to help out please comment here claiming which ones you will work on. I will start at the top and work my way down skipping any that are claimed. While we do not explicitly call out the use of
### Security
in our changelog guidelines there is mention of such in the keep a changelog guidelines which we are based on.Github is the best:![image](https://user-images.githubusercontent.com/3145127/33033410-252a6c2c-cdd9-11e7-886b-9390291068b2.png)
Here is a list of plugins: