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

Fix warning from upstream rubocop #57

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions .rubocop.yml
@@ -1,5 +1,8 @@
inherit_from: ./config/default.yml

require:
- rubocop-performance

Choose a reason for hiding this comment

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

Are some of the renamed checks in Rubocop Performance, or is this a separate change?

Copy link
Author

Choose a reason for hiding this comment

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

hmm, we are using some Performance cops in the .rubocop.yml of rubocop-github itself, and I got errors when rubocop-performance is not required


Naming/FileName:
Enabled: true
Exclude:
Expand Down
2 changes: 1 addition & 1 deletion .travis.yml
Expand Up @@ -6,7 +6,7 @@ git:
depth: 10

rvm:
- 2.3.8
- 2.4.6
- 2.5.5
- 2.6.3
- 2.7.1

Choose a reason for hiding this comment

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

For future debugging purposes, can we put CI changes in a separate PR?

Copy link
Author

Choose a reason for hiding this comment

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

sure, let me bring it in separate PR then

Copy link
Author

@ipmsteven ipmsteven Apr 20, 2020

Choose a reason for hiding this comment

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

Choose a reason for hiding this comment

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

It looks like we'll need to do this PR first, then the CI PR since CI is breaking on the warnings :)

Copy link
Author

Choose a reason for hiding this comment

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

yes, but I got CI test failure on 2.3 for this PR :(

Choose a reason for hiding this comment

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

It looks like on 2.3 it failed with Layout/IndentationStyle:

https://travis-ci.org/github/github/rubocop-github/jobs/676500683

Which is odd, since you fixed that. I didn't chase it further (maybe there's another place where it's being used?)

2 changes: 2 additions & 0 deletions Gemfile
Expand Up @@ -2,3 +2,5 @@

source "https://rubygems.org"
gemspec

gem "rubocop-performance", require: false
15 changes: 6 additions & 9 deletions config/_default_shared.yml
Expand Up @@ -26,9 +26,15 @@ Layout/EndAlignment:
Layout/EndOfLine:
Enabled: true

Layout/IndentationStyle:
Enabled: true

Layout/InitialIndentation:
Enabled: true

Layout/LineLength:
Enabled: false

Layout/SpaceAfterColon:
Enabled: true

Expand Down Expand Up @@ -72,9 +78,6 @@ Layout/SpaceInsideRangeLiteral:
Layout/SpaceInsideReferenceBrackets:
Enabled: true

Layout/Tab:
Enabled: true

Layout/TrailingEmptyLines:
Enabled: true

Expand Down Expand Up @@ -108,9 +111,6 @@ Lint/EmptyEnsure:
Lint/EmptyInterpolation:
Enabled: true

Lint/EndInMethod:
Enabled: true

Lint/EnsureReturn:
Enabled: true

Expand Down Expand Up @@ -183,9 +183,6 @@ Metrics/ClassLength:
Metrics/CyclomaticComplexity:
Enabled: false

Metrics/LineLength:
Enabled: false

Metrics/MethodLength:
Enabled: false

Expand Down