Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

rubocop: Trim exclude paths without offenses; move some more config #14878

Merged
merged 4 commits into from
Mar 6, 2023
Merged

rubocop: Trim exclude paths without offenses; move some more config #14878

merged 4 commits into from
Mar 6, 2023

Conversation

issyl0
Copy link
Member

@issyl0 issyl0 commented Mar 3, 2023 •
edited
Loading

  • 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?
  • Part of Remove usage of Exclude with rubocop #14685.
  • rubocop: Trim Style/BlockDelimiters exclude paths without offenses
  • rubocop: Move AllowedMethods inheritance to the cop it's used in
  • rubocop: Switch to Cask-specific Style/ClassVars in-line disables

@BrewTestBot
Copy link
Member

Review period will end on 2023-03-06 at 22:56:15 UTC.

@BrewTestBot BrewTestBot added the waiting for feedback Merging is blocked until sufficient time has passed for review label Mar 3, 2023
@issyl0 issyl0 changed the title A little more rubocop tidying rubocop: Trim exclude paths without offenses; move some more config Mar 3, 2023
@issyl0 issyl0 mentioned this pull request Mar 4, 2023
Merged
9 tasks
issyl0 added 4 commits March 4, 2023 00:48
@issyl0
rubocop: Move AllowedMethods inheritance to the cop it's used in
0a18898 
- `Include` and `Exclude` are ubiquitous but `AllowedMethods` less so.
- Move the inheritance to the single cop configuration that it's used
  in. Like we do with some other cop configs where they have default
  paths/settings that we want to add to, not replace.
@issyl0
rubocop: Trim Style/BlockDelimiters exclude paths without offenses
87b94d9
@issyl0
rubocop: In-line disables for Style/{Class,Global}Vars
f2e21d5 
- Other occurrences of `@@` and `$` variables are individually disabled
  in-line in other files.
- So let's follow the same pattern for Homebrew/homebrew-cask developer
  scripts, since there are only a few.
@issyl0
rubocop: Further alphabetization, M comes before N
72a694d
@issyl0 issyl0 force-pushed the a-little-more-rubocop-tidying branch from 86ccd1c to 72a694d Compare March 4, 2023 00:50
@MikeMcQuaid MikeMcQuaid added the critical Critical change which should be shipped as soon as possible. label Mar 6, 2023
@BrewTestBot BrewTestBot removed the waiting for feedback Merging is blocked until sufficient time has passed for review label Mar 6, 2023
@BrewTestBot
Copy link
Member

Review period skipped due to critical label.

MikeMcQuaid
MikeMcQuaid approved these changes Mar 6, 2023
View reviewed changes
Copy link
Member

@MikeMcQuaid MikeMcQuaid left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great, thanks!

Would be great if there was some way of auto-removing these Exclude 🤔. Not at all blocking but potentially a cool project!

@MikeMcQuaid MikeMcQuaid merged commit b05bd61 into Homebrew:master Mar 6, 2023
@github-actions github-actions bot added the outdated PR was locked due to age label Apr 6, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 6, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@MikeMcQuaid MikeMcQuaid MikeMcQuaid approved these changes

@BrewTestBot BrewTestBot BrewTestBot approved these changes

Assignees
No one assigned
Labels
critical Critical change which should be shipped as soon as possible. outdated PR was locked due to age
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@issyl0 @BrewTestBot @MikeMcQuaid