Simplify setting default cops

dafyddcrosby commented 10 months ago

Description

tl;dr Similar to https://github.com/chef/chefstyle/pull/157, stop vendoring the RuboCop configurations directly.

I noticed a bunch of stale cop names in cookstyle.yml, and saw how manually intensive and error-prone the process is to update RuboCop in Cookstyle. Several of the cops were renamed and Cookstyle stopped checking for them - the current vendoring process didn't stop that. In https://github.com/facebook/chef-cookbooks we also take an allowlist-style approach to RuboCop lints, but use DisableByDefault to accomplish that. There's prior art here: https://github.com/chef/chefstyle/pull/66 took a stab at this, but there were other changes mixed in besides that transition, and it doesn't look like there was a parity test of the configuration after the change. Exactly what prompted the revert wasn't written down (in the repo, at least).

This is an attempt at simplifying Cookstyle so that we don't need to vendor RuboCop configs. I put care into providing the step-by-step in each commit comment, including why several cops are now commented out in cookstyle.yml.

Types of changes

[ ] Bug fix (non-breaking change which fixes an issue)
[x] New feature (non-breaking change which adds functionality)
[ ] Breaking change (fix or feature that would cause existing functionality to change)
[ ] Chore (non-breaking change that does not add functionality or fix an issue)

Checklist:

[x] I have read the CONTRIBUTING document.
[x] I have run the pre-merge tests locally and they pass.
[x] I have updated the documentation accordingly.
[ ] I have added tests to cover my changes.
[ ] If Gemfile.lock has changed, I have used --conservative to do it and included the full output in the Description above.
[x] All new and existing tests passed.
[x] All commits have been signed-off for the Developer Certificate of Origin.

tas50 commented 10 months ago

@dafyddcrosby after your chefstyle PR I implemented a similar cookstyle change with Rubocop bumped and new cops enabled. Glad to see you fixed the annoying warnings about the chef version https://github.com/tas50/cookstyle/commit/78c0346c3c4310b391ac5ebca6d6bf4e8bed3eac

dafyddcrosby commented 10 months ago

@dafyddcrosby after your chefstyle PR I implemented a similar cookstyle change with Rubocop bumped and new cops enabled. Glad to see you fixed the annoying warnings about the chef version tas50@78c0346

Nice! My original plan was to do the two simplification diffs, move chefstyle.yml and the lints in that repo into this one (since they're still the same RuboCop), migrate the chefstyle gem references to cookstyle, and then we could deprecate and archive chefstyle entirely. I like the idea of doing a major version bump for the engine upgrade and rule name fix-up along with the chefstyle move, since it's Sufficiently Major™️. It looks like there's a few orthogonal changes in your commit that we could split out and land before the major version bump, too.

cc @tpowell-progress let's discuss this today in code review today, I'm thinking I'll address the current rubocop nits on this PR and I/we can rebase your commit onto this?