rubocop: Some more config cleanup

[issyl0]

• Have you followed the guidelines in our Contributing document?
• Have you checked to ensure there aren't other open Pull Requests for the same change?
• Have you added an explanation of what your changes do and why you'd like us to include them?
• Have you written new tests for your changes? Here's an example.
• Have you successfully run brew style with your changes locally?
• Have you successfully run brew typecheck with your changes locally?
• Have you successfully run brew tests with your changes locally?

---

• Found on my travels through Remove usage of Exclude with rubocop #14685.
• Clean up non-existent files/dirs from excludes.
• Naming/PredicateName allows is_a? by default.
• The test dir is a default exclude for Style/Documentation.

[BrewTestBot]

Review period will end on 2023-02-21 at 00:00:00 UTC.

[issyl0]

🤔 Why are things in test/**/* failing Style/Documentation on CI? Locally the cop shows no offenses.

[Bo98]

🤔 Why are things in test/**/* failing Style/Documentation on CI? Locally the cop shows no offenses.

I think the PWD where you run brew style can affect its output sometimes. (It obviously shouldn't but I have seen it happen nevertheless.)

[MikeMcQuaid]

Thanks again @issyl0!

[BrewTestBot]

Review period skipped due to critical label.

[dduugg]

🤔 Why are things in test/**/* failing Style/Documentation on CI? Locally the cop shows no offenses.

I think the PWD where you run brew style can affect its output sometimes. (It obviously shouldn't but I have seen it happen nevertheless.)

Specifically, I think this has to do with whether rubocop is invoked with config https://github.com/Homebrew/brew/blob/master/Library/.rubocop.yml
or
https://github.com/Homebrew/brew/blob/master/Library/Homebrew/.rubocop.yml

I'm looking into consolidating them into a single file.

[MikeMcQuaid]

Specifically, I think this has to do with whether rubocop is invoked with config https://github.com/Homebrew/brew/blob/master/Library/.rubocop.yml or https://github.com/Homebrew/brew/blob/master/Library/Homebrew/.rubocop.yml

I'm looking into consolidating them into a single file.

We have separate rules for things in Library/Taps and for the Homebrew/brew codebase. As you’ve probably seen in other issues: specifying paths in .rubocop.yml doesn’t seem to be terribly reliable here so we rely on separate configurations. The style varies a fair bit between the two, deliberately, as Library/Taps are formula/cask DSL(s) and Homebrew/brew is a fairly standard Ruby project.

[dduugg]

Specifically, I think this has to do with whether rubocop is invoked with config https://github.com/Homebrew/brew/blob/master/Library/.rubocop.yml or https://github.com/Homebrew/brew/blob/master/Library/Homebrew/.rubocop.yml
I'm looking into consolidating them into a single file.

We have separate rules for things in Library/Taps and for the Homebrew/brew codebase. As you’ve probably seen in other issues: specifying paths in .rubocop.yml doesn’t seem to be terribly reliable here so we rely on separate configurations. The style varies a fair bit between the two, deliberately, as Library/Taps are formula/cask DSL(s) and Homebrew/brew is a fairly standard Ruby project.

This is useful context. It's been confusing triggering failures in other repos when making updates to the rubocop config here. Another approach could be to have a centralized, versioned rubocop repo, and each brew repo can independently use that config and a dedicated github action to run rubocop (and this repo could still enable the add'l config options that are currently in a separate file). (It looks like Shopify takes this approach: https://rubygems.org/gems/rubocop-shopify )

[issyl0]

@dduugg Yeah, GitHub has the "separate repo, versioned gem" approach too - github/rubocop-github - it's a common pattern. Might be worth an issue to track this separately as a future enhancement us?

[issyl0]

I'm going to undo the Style/Documentation test/** exclude removal since it's not working. No amount of massaging

inherit_mode:
  merge:
  - Exclude

works when RuboCop is run from the specific path. 🤔

[MikeMcQuaid]

It's been confusing triggering failures in other repos when making updates to the rubocop config here.

I think given that we don't "use RuboCop" as much as we "use brew style that happens to use RubocCop (and shellcheck): it makes more sense for this to live here.

I appreciate that it's a little confusing to have to change other repos to have CI pass here but I think it's better for us to have this a big more centralised and blocking.

[MikeMcQuaid]

I'm going to undo the Style/Documentation test/** exclude removal since it's not working.

@issyl0 IMO we should just remove it as a requirement and instead flip it to only be required on specific classes that are part of our public API.

[issyl0]

@issyl0 IMO we should just remove it as a requirement and instead flip it to only be required on specific classes that are part of our public API.

@MikeMcQuaid Yeah, will do this soon with the metrics stuff too.