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.

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

.rubocop: tweak rules. #4797

Merged
merged 1 commit into from Sep 2, 2018
Merged

Conversation

MikeMcQuaid
Copy link
Member

Adjust the rules based on the current codebase. Remove various enable, disables and default values that are unnecessary. Add more comments explaining why. Make minor changes needed to enable a few more rules.

  • 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 tests with your changes locally?

@ghost ghost assigned MikeMcQuaid Sep 2, 2018
@ghost ghost added the in progress Maintainers are working on this label Sep 2, 2018
@MikeMcQuaid MikeMcQuaid changed the title Homebrew/.rubocop: tweak rules. .rubocop: tweak rules. Sep 2, 2018
@MikeMcQuaid
Copy link
Member Author

With these changes there's only a single complaint in homebrew/core which will be fixed in Homebrew/homebrew-core#31709

Layout/IndentHeredoc:
EnforcedStyle: squiggly

# conflicts with DSL-style path concatenation with `/`
Layout/SpaceAroundOperators:
Enabled: false

# favor parens-less DSL-style arguments
# # favor parens-less DSL-style arguments
Copy link
Member

Choose a reason for hiding this comment

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

Is the second # a typo?

(Also favor? 🇺🇸 👀, heh)

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, fixing (both 😉)

Copy link
Member

Choose a reason for hiding this comment

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

There's a few of both here but I won't point them all out to avoid you wanting to prod me with a stick, justifiably 😄

Metrics/PerceivedComplexity:
# GitHub diff UI wraps beyond 118 characters (so that's the goal)
Metrics/LineLength:
Max: 189
Copy link
Member

Choose a reason for hiding this comment

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

Wouldn't object to knocking this down a bit, FWIW. I have a file locally of all the core formulae that have overly long lines, which I've started working through. I think 150 is doable fairly quickly.

MikeMcQuaid added a commit to Homebrew/homebrew-core that referenced this pull request Sep 2, 2018
Soon to be flagged by `brew style` in Homebrew/brew#4797

Closes #31709.

Signed-off-by: Mike McQuaid <mike@mikemcquaid.com>
Adjust the rules based on the current codebase. Remove various enable,
disables and default values that are unnecessary. Add more comments
explaining why. Make minor changes needed to enable a few more rules.
@MikeMcQuaid
Copy link
Member Author

Wouldn't object to knocking this down a bit, FWIW. I have a file locally of all the core formulae that have overly long lines, which I've started working through. I think 150 is doable fairly quickly.

@DomT4 I'm game to take it down the whole way at some point but yeh: any help before that'd be welcome 👍

@DomT4
Copy link
Member

DomT4 commented Sep 2, 2018

Aye, I think periodically reducing the number is a solid way to go. After Homebrew/homebrew-core#31712 is folded in (it'll fail CI, but for previously known reasons) we'll be down to 94 non-exempt lines over 150 characters in core.

@MikeMcQuaid MikeMcQuaid merged commit 3b91c5f into Homebrew:master Sep 2, 2018
@ghost ghost removed the in progress Maintainers are working on this label Sep 2, 2018
@MikeMcQuaid MikeMcQuaid deleted the rubocop-tweaks branch September 2, 2018 20:13
@lock lock bot added the outdated PR was locked due to age label Oct 2, 2018
@lock lock bot locked as resolved and limited conversation to collaborators Oct 2, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
outdated PR was locked due to age
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants