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

Bump rubocop to 0.71 #36426

Merged
merged 1 commit into from Jun 6, 2019

Conversation

abhaynikam
Copy link
Contributor

Rubocop channel 0.71 -> codeclimate/codeclimate-rubocop#183

Shoud we update the gemfile as well with gem "rubocop", "~> 0.71.0", require: false?

@yahonda
Copy link
Member

yahonda commented Jun 6, 2019

I think you will need to update Gemfile and .rubocop.yml file. I'm preparing a similar change at another repository https://github.com/rsim/oracle-enhanced/pull/1887/files

I rather prefer @koic or someone who is familiar with RuboCop makes this kind of change.

@abhaynikam
Copy link
Contributor Author

@yahonda Alright. @koic or someone who is familiar can pick this up. I'll just try one push and close the PR since I spent sometime on it 😄 . Hope that is fine.

@yahonda
Copy link
Member

yahonda commented Jun 6, 2019

Great. I hope your next one should be fine.

@abhaynikam
Copy link
Contributor Author

@yahonda : Everything passed here. Ping me if I should close this PR.

@@ -295,7 +295,7 @@ class TestBase < ActiveSupport::TestCase
10.times do |x|
controller = WithString.new
controller.define_singleton_method :index do
render template: ActionView::Template::Text.new("Hello string!"), locals: { :"x#{x}" => :omg }
render template: ActionView::Template::Text.new("Hello string!"), locals: { "x#{x}" => :omg }
Copy link
Member

Choose a reason for hiding this comment

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

Which cop does this violate?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Layout/EmptyLinesAroundBlockBody -> https://github.com/rails/rails/blob/master/.rubocop.yml#L65

This was changed in rubocop/rubocop#7090 I think

Copy link
Member

Choose a reason for hiding this comment

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

I asked why rubocop correct intentional symbol key to string key wrongly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@kamipo : My bad. I wanted to comment it for another comment of your(#36426 (comment)).

Style/HashSyntax cop fails here. It should be following as per -> rubocop/rubocop#6948

Rubocop new syntax when keys are interpolated.

"x#{x}": :omg

I'll make that change.

Copy link
Contributor

Choose a reason for hiding this comment

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

This difference is due to the following changes.
rubocop/rubocop#6948

@@ -786,19 +786,17 @@ def test_eager_with_has_many_and_limit_and_conditions_on_the_eagers
def test_eager_with_has_many_and_limit_and_scoped_conditions_on_the_eagers
posts = nil
Post.includes(:comments)
.where("comments.body like 'Normal%' OR comments.#{QUOTED_TYPE}= 'SpecialComment'")
Copy link
Member

Choose a reason for hiding this comment

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

ditto.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

Don't apply your indentation edit.

% be rubocop -a activerecord/test/cases/associations/eager_test.rb
Inspecting 1 file
C

Offenses:

activerecord/test/cases/associations/eager_test.rb:792:1: C: [Corrected] Layout/EmptyLinesAroundBlockBody: Extra empty line detected at block body beginning.
activerecord/test/cases/associations/eager_test.rb:801:1: C: [Corrected] Layout/EmptyLinesAroundBlockBody: Extra empty line detected at block body beginning.

1 file inspected, 2 offenses detected, 2 offenses corrected
% git diff activerecord/test/cases/associations/eager_test.rb
diff --git a/activerecord/test/cases/associations/eager_test.rb b/activerecord/test/cases/associations/eager_test.rb
index f7aad9d775..42af1ac8d8 100644
--- a/activerecord/test/cases/associations/eager_test.rb
+++ b/activerecord/test/cases/associations/eager_test.rb
@@ -789,7 +789,6 @@ def test_eager_with_has_many_and_limit_and_scoped_conditions_on_the_eagers
       .where("comments.body like 'Normal%' OR comments.#{QUOTED_TYPE}= 'SpecialComment'")
       .references(:comments)
       .scoping do
-
       posts = authors(:david).posts.limit(2).to_a
       assert_equal 2, posts.size
     end
@@ -798,7 +797,6 @@ def test_eager_with_has_many_and_limit_and_scoped_conditions_on_the_eagers
       .where("authors.name = 'David' AND (comments.body like 'Normal%' OR comments.#{QUOTED_TYPE}= 'SpecialComment')")
       .references(:authors, :comments)
       .scoping do
-
       count = Post.limit(2).count
       assert_equal count, posts.size
     end

@@ -680,8 +680,7 @@ class RequestProtocol < BaseRequestTest

class RequestMethod < BaseRequestTest
test "method returns environment's request method when it has not been
overridden by middleware".squish do
Copy link
Member

Choose a reason for hiding this comment

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

ditto.

Copy link
Member

Choose a reason for hiding this comment

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

% be rubocop -a actionpack/test/dispatch/request_test.rb
Inspecting 1 file
C

Offenses:

actionpack/test/dispatch/request_test.rb:684:1: C: [Corrected] Layout/EmptyLinesAroundBlockBody: Extra empty line detected at block body beginning.

1 file inspected, 1 offense detected, 1 offense corrected
% git diff actionpack/test/dispatch/request_test.rb
diff --git a/actionpack/test/dispatch/request_test.rb b/actionpack/test/dispatch/request_test.rb
index eb49396145..0ec8dd25e0 100644
--- a/actionpack/test/dispatch/request_test.rb
+++ b/actionpack/test/dispatch/request_test.rb
@@ -681,7 +681,6 @@ class RequestProtocol < BaseRequestTest
 class RequestMethod < BaseRequestTest
   test "method returns environment's request method when it has not been
     overridden by middleware".squish do
-
     ActionDispatch::Request::HTTP_METHODS.each do |method|
       request = stub_request("REQUEST_METHOD" => method)
 

.rubocop.yml Outdated
@@ -77,7 +79,7 @@ Layout/EmptyLinesAroundMethodBody:
Layout/EmptyLinesAroundModuleBody:
Enabled: true

Layout/FirstParameterIndentation:
Layout/IndentFirstArgument:
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it is preferable to move in ASCII order.

Gemfile Outdated
@@ -28,8 +28,9 @@ gem "uglifier", ">= 1.3.0", require: false
# Explicitly avoid 1.x that doesn't support Ruby 2.4+
gem "json", ">= 2.0.0"

gem "rubocop", ">= 0.47", require: false
gem "rubocop", "~> 0.71.0", require: false
Copy link
Contributor

Choose a reason for hiding this comment

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

rails/rails repository is maintained by Gemfile.lock. Is this change meaningful?

@koic
Copy link
Contributor

koic commented Jun 6, 2019

I think that Rails: Enabled: true can be removed from .rubocop.yaml.

diff --git a/.rubocop.yml b/.rubocop.yml
index 18c20776c6..22161b1e16 100644
--- a/.rubocop.yml
+++ b/.rubocop.yml
@@ -20,9 +20,6 @@ Performance:
   Exclude:
     - '**/test/**/*'

-Rails:
-  Enabled: true
-
 # Prefer assert_not over assert !
 Rails/AssertNot:
   Include:

It is enabled when require: rubocop-rails is specified in .rubocop.yml.
https://github.com/rubocop-hq/rubocop/pull/7093/files#diff-68891125561f1767ea375a89b05f7e1e

@abhaynikam abhaynikam force-pushed the bump-codeclimate-rubocop-version branch 2 times, most recently from 3420cf8 to fa24346 Compare June 6, 2019 09:47
@abhaynikam abhaynikam force-pushed the bump-codeclimate-rubocop-version branch from fa24346 to 00b3b68 Compare June 6, 2019 10:05
@abhaynikam
Copy link
Contributor Author

@kamipo : Done with requested changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants