Upgrade rubocop to 0.48 #8652

giginet · 2017-03-26T14:35:49Z

Checklist

I've run bundle exec rspec from the root directory to see all new and existing tests pass
I've followed the fastlane code style and run bundle exec rubocop -a to ensure the code style is valid
I've read the Contribution Guidelines
I've updated the documentation if necessary.

Description

Updated rubocop to latest version and .rubocop.yml.

Motivation and Context

rubocop 0.48 has been released.
Because of this change, CI is failing.
#8649

So I updated rubocop and .rubocop.yml.

giginet

I added settings about new cops.
Let's discuss this.

giginet · 2017-03-26T14:38:53Z

.rubocop.yml

@@ -81,6 +81,16 @@ Style/DotPosition:
 Style/DoubleNegation:
  Enabled: false

+Style/SymbolArray:


SymbolArray cop is enabled by default.
This cop changes all [] into %i.
It has big effect on entire project.
So I disabled.

ref:rubocop/rubocop#4124

This explanation is great. Can you add it as a comment above the config item so we can easily see why we did it?

giginet · 2017-03-26T14:39:53Z

.rubocop.yml

+Style/SymbolArray:
+  Enabled: false
+
+Style/IndentHeredoc:


IndentHeredoc is introduced from Ruby 2.3.
However, we have to support Ruby 2.0.0.
So I disabled.

ref: rubocop/rubocop#4028

This explanation is great. Can you add it as a comment above the config item so we can easily see why we did it?

giginet · 2017-03-26T14:41:38Z

.rubocop.yml

+    - '**/Rakefile'
+    - '**/Fastfile'
+    - '**/Deliverfile'
+    - '**/Snapfile'


These files are checked by default.
So I ignored.

https://github.com/bbatsov/rubocop/pull/3997/files#diff-af441552566679e9498e0a466229a484R32

giginet · 2017-03-26T14:43:09Z

.rubocop.yml

+Style/IndentHeredoc:
+  Enabled: false
+
+Style/MixinGrouping:


ref: rubocop/rubocop#3936

In latest rubocop, this cop detects include of rspec matcher as Module::include.
This behavior is strange. It would be the issue on rubocop.
So I disabled.

See this rubocop/rubocop#4172

giginet · 2017-03-26T14:45:15Z

.rubocop.yml

@@ -89,6 +99,9 @@ Lint/HandleExceptions:
 Lint/UnusedBlockArgument:
  Enabled: false

+Lint/AmbiguousBlockAssociation:


ref rubocop/rubocop@710d0e6

I prefer AmbiguousBlockAssociation is enabled 👍

If this cop is enabled, autocorrect seems not to work.
Should we fix them manually?

giginet · 2017-03-26T14:49:18Z

fastlane/lib/fastlane/actions/commit_version_bump.rb

@@ -58,8 +58,8 @@ def self.run(params)
        # Removes .plist files that matched the given expression in the 'ignore' parameter
        ignore_expression = params[:ignore]
        if ignore_expression
-          info_plist_files.select! do |info_plist_file|
-            !info_plist_file.match(ignore_expression)
+          info_plist_files.reject! do |info_plist_file|


These changes are caused by InverseMethod cop.
https://github.com/bbatsov/rubocop/blob/c06b2e34d1a6108e03cd5c03761db28167005829/manual/cops_style.md#styleinversemethods

I think it should be enabled.

yeah that looks fine 👍

nafu

Looks good to me 👍
We need more approval ✅

KrauseFx

This looks great, thanks @giginet for this 👍 Please add the comments to the .rubocop.yml, then it's ready to be merged. Also, can you run the test suite on Ruby 2.0 to make sure everything works as expected?

KrauseFx · 2017-03-27T01:36:55Z

fastlane/lib/fastlane/actions/commit_version_bump.rb

@@ -58,8 +58,8 @@ def self.run(params)
        # Removes .plist files that matched the given expression in the 'ignore' parameter
        ignore_expression = params[:ignore]
        if ignore_expression
-          info_plist_files.select! do |info_plist_file|
-            !info_plist_file.match(ignore_expression)
+          info_plist_files.reject! do |info_plist_file|


yeah that looks fine 👍

KrauseFx · 2017-03-27T01:37:13Z

fastlane/lib/fastlane/actions/oclint.rb

@@ -43,7 +43,7 @@ def self.run(params)
          file_ruby = file.gsub('\ ', ' ')
          File.exist?(file_ruby) and
            (!select_regex or file_ruby =~ select_regex) and
-            (!exclude_regex or !(file_ruby =~ exclude_regex))
+            (!exclude_regex or file_ruby !~ exclude_regex)


Does this syntax also work with Ruby 2.0?

!~ operator is available since 1.9. So we can use this on Apple Ruby.

Awesome, sounds good 👍

giginet · 2017-03-27T06:20:02Z

I ran rspec and rubocop on '2.0.0-p648'. And they are passed fine 👍

giginet · 2017-03-27T10:19:30Z

Thanks for reviewing!

fastlane-bot · 2017-03-28T04:17:31Z

Hey @giginet 👋

Thank you for your contribution to fastlane and congrats on getting this pull request merged 🎉
The code change now lives in the master branch, however it wasn't released yet to RubyGems.
We usually ship about once a week, and your PR will be included in the next one.

Please let us know if this change requires an immediate release by adding a comment here 👍
We'll notify you once we shipped a new release with your changes 🚀

giginet added 3 commits March 26, 2017 23:27

Upgrade rubocop

a22be27

Modify .rubocop.yml

69e08f7

Apply rubocop -a

0b2070f

googlebot added the cla: yes label Mar 26, 2017

giginet commented Mar 26, 2017

View reviewed changes

nafu approved these changes Mar 26, 2017

View reviewed changes

KrauseFx approved these changes Mar 27, 2017

View reviewed changes

Add comment to .rubocop.yml

6281a8d

giginet force-pushed the rubocop48 branch from 21a0b99 to 6281a8d Compare March 27, 2017 06:24

mfurtak approved these changes Mar 27, 2017

View reviewed changes

giginet merged commit bbe0f81 into fastlane:master Mar 27, 2017

giginet deleted the rubocop48 branch March 27, 2017 10:20

fastlane-bot added the status: included-in-next-release label Mar 28, 2017

KrauseFx mentioned this pull request Mar 30, 2017

Version bump to 2.24.0 #8709

Merged

fastlane locked and limited conversation to collaborators Jun 26, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Upgrade rubocop to 0.48 #8652

Upgrade rubocop to 0.48 #8652

giginet commented Mar 26, 2017

giginet left a comment

giginet Mar 26, 2017 •

edited

mfurtak Mar 26, 2017

giginet Mar 26, 2017

mfurtak Mar 26, 2017

giginet Mar 26, 2017

giginet Mar 26, 2017

giginet Mar 27, 2017

giginet Mar 27, 2017

giginet Mar 26, 2017

nafu Mar 26, 2017

giginet Mar 27, 2017

giginet Mar 26, 2017

KrauseFx Mar 27, 2017

nafu left a comment

KrauseFx left a comment

KrauseFx Mar 27, 2017

KrauseFx Mar 27, 2017

giginet Mar 27, 2017

KrauseFx Mar 27, 2017

giginet commented Mar 27, 2017

giginet commented Mar 27, 2017

fastlane-bot commented Mar 28, 2017

Upgrade rubocop to 0.48 #8652

Upgrade rubocop to 0.48 #8652

Conversation

giginet commented Mar 26, 2017

Checklist

Description

Motivation and Context

giginet left a comment

Choose a reason for hiding this comment

giginet Mar 26, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nafu left a comment

Choose a reason for hiding this comment

KrauseFx left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

giginet commented Mar 27, 2017

giginet commented Mar 27, 2017

fastlane-bot commented Mar 28, 2017

giginet Mar 26, 2017 •

edited