Enable Rubocop cop to omit parentheses from one-line method calls #2778

quartzmo · 2019-01-07T23:36:28Z

This PR currently applies this change only to google-cloud-logging. If the style is acceptable, please approve and I will apply to all manual gems, then request a full review.

Enable Style/MethodCallWithArgsParentheses omit_parentheses

[closes #2716]

blowmage · 2019-01-07T23:41:39Z

This reads fine to me but I'm sure others will disagree. @dazuma @TheRoyalTnetennba can you weigh in on this?

quartzmo · 2019-01-07T23:43:08Z

I'm OK with whatever style we agree on, but it seems to me it should be enforced.

TheRoyalTnetennba

LGTM

google-cloud-logging/lib/google/cloud/logging/entry.rb

@@ -473,7 +473,7 @@ def labels_grpc
          return {} if labels.nil?
          # Coerce symbols to strings
          Hash[labels.map do |k, v|
-            v = String(v) if v.is_a? Symbol
+            v = String v if v.is_a? Symbol


quartzmo · 2019-01-21T23:07:16Z

It looks like rubocop/rubocop#6643 is likely to be merged, so I'm leaving this PR open. Will check again on Rubocop progress next week.

dazuma · 2019-01-22T06:35:56Z

rubocop/rubocop#6643 is merged. As it is a new feature, it will likely get released in rubocop version 0.64.0.

quartzmo · 2019-01-22T20:08:30Z

@blowmage @dazuma I temporarily switched the Rubocop dependency to get @dazuma's new feature from rubocop/rubocop#6643:

gem "rubocop", git: "git@github.com:rubocop-hq/rubocop.git", branch: "master"

I then reverted the Kernel method calls to their original style, with parentheses.

PTAL at the current style and let me know if it looks good. I'd be happy to do an additional gem such as BigQuery if it helps.

quartzmo · 2019-01-22T20:11:48Z

@dazuma Your new feature AllowParenthesesInCamelCaseMethod works, and rubocop succeeds. If I remove AllowParenthesesInCamelCaseMethod, it fails on the Kernel methods. However, when I run rubocop I see this warning:

Using rubocop 0.63.1 from git@github.com:rubocop-hq/rubocop.git (at master@d6e7a16)
...
Running RuboCop...
Warning: Style/MethodCallWithArgsParentheses does not support AllowParenthesesInCamelCaseMethod parameter.

Supported parameters are:

  - Enabled
  - IgnoreMacros
  - IgnoredMethods
  - AllowParenthesesInMultilineCall
  - AllowParenthesesInChaining
  - EnforcedStyle
  - SupportedStyles

Inspecting 26 files
..........................

26 files inspected, no offenses detected

dazuma · 2019-01-22T22:36:59Z

@quartzmo Ah crap. That check slipped through while testing. Fix: rubocop/rubocop#6697

dazuma · 2019-01-22T22:38:51Z

@quartzmo If you run against my branch, does the warning go away?

gem "rubocop", git: "git@github.com:dazuma/rubocop.git", branch: "paren-param"

quartzmo · 2019-01-23T00:05:49Z

@dazuma Looks good!

Using rubocop 0.63.1 from git@github.com:dazuma/rubocop.git (at paren-param@a5a471a)
...
Chriss-iMac:google-cloud-logging quartzmo$ b rake rubocop
Running RuboCop...
Inspecting 26 files
..........................

26 files inspected, no offenses detected

dazuma · 2019-01-23T19:04:01Z

FYI the fix is now on rubocop master.

quartzmo · 2019-01-23T19:42:49Z

@blowmage @dazuma Does the style in this PR now look good? Should I add another gem (for example, google-cloud-bigquery) to the PR to be sure?

blowmage · 2019-01-23T21:09:01Z

The style looks good to me. 👍 for applying it to more gems.

quartzmo · 2019-02-08T21:30:36Z

Added google-cloud-bigquery.

google-cloud-bigquery/lib/google/cloud/bigquery/dataset.rb

google-cloud-bigquery/lib/google/cloud/bigquery/job.rb

google-cloud-bigquery/lib/google/cloud/bigquery/table.rb

google-cloud-bigquery/lib/google/cloud/bigquery/table/async_inserter.rb

quartzmo · 2019-02-13T19:26:11Z

Good news, Rubocop 0.64.0 was released with @dazuma's change.

Enable Style/MethodCallWithArgsParentheses omit_parentheses [closes googleapis#2716]

Update .rubocop.yml to use AllowParenthesesInCamelCaseMethod

Add Style/MethodCallWithArgsParentheses.

quartzmo · 2019-02-14T20:24:07Z

Rebased to use rubocop 0.64.0 from master.

google-cloud-bigquery/lib/google/cloud/bigquery/convert.rb

Add Style/MethodCallWithArgsParentheses.

google-cloud-bigquery/lib/google/cloud/bigquery/dataset.rb

googlebot added the cla: yes This human has signed the Contributor License Agreement. label Jan 7, 2019

quartzmo self-assigned this Jan 7, 2019

quartzmo requested a review from dazuma January 7, 2019 23:36

quartzmo assigned blowmage and unassigned blowmage Jan 7, 2019

quartzmo requested a review from blowmage January 7, 2019 23:36

quartzmo added the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Jan 7, 2019

quartzmo requested a review from TheRoyalTnetennba January 7, 2019 23:43

TheRoyalTnetennba approved these changes Jan 7, 2019

View reviewed changes

dazuma reviewed Jan 8, 2019

View reviewed changes

quartzmo force-pushed the rubocop-parens branch from 09e09dd to ab2d5aa Compare January 22, 2019 20:04

JustinBeckwith added the 🚨 This issue needs some love. label Feb 7, 2019

yoshi-automation added 🚨 This issue needs some love. and removed 🚨 This issue needs some love. labels Feb 7, 2019

quartzmo force-pushed the rubocop-parens branch from ab2d5aa to e893169 Compare February 8, 2019 21:29

blowmage suggested changes Feb 8, 2019

View reviewed changes

quartzmo added 10 commits February 14, 2019 12:09

Update .rubocop.yml

e66dcb4

Enable Style/MethodCallWithArgsParentheses omit_parentheses [closes googleapis#2716]

Ignore kernel methods

51211df

Fix Rubocop parens errors after rebase.

aa6fcd4

Update Rubocop dependency to git master

0cd46a2

Update .rubocop.yml to use AllowParenthesesInCamelCaseMethod

Update google-cloud-bigquery/.rubocop.yml

fb1e897

Add Style/MethodCallWithArgsParentheses.

Update google-cloud-core/.rubocop.yml

501a28c

Add Style/MethodCallWithArgsParentheses.

Update google-cloud-datastore/.rubocop.yml

f893c14

Add Style/MethodCallWithArgsParentheses.

Update google-cloud-debugger/.rubocop.yml

b115c11

Add Style/MethodCallWithArgsParentheses.

Update google-cloud-env/.rubocop.yml

014070d

Add Style/MethodCallWithArgsParentheses.

Restore rubocop 0.61.0 before rebase

7388b8e

quartzmo force-pushed the rubocop-parens branch from 1dd4046 to 7388b8e Compare February 14, 2019 19:23

blowmage reviewed Feb 15, 2019

View reviewed changes

google-cloud-bigquery/lib/google/cloud/bigquery/convert.rb Outdated Show resolved Hide resolved

Add Bigquery::Convert.millis_to_time

dde9561

quartzmo force-pushed the rubocop-parens branch from 21a4f68 to dde9561 Compare February 15, 2019 18:47

quartzmo added 6 commits February 15, 2019 12:08

Update google-cloud-error_reporting/.rubocop.yml

2159e07

Add Style/MethodCallWithArgsParentheses.

Update google-cloud-firestore/.rubocop.yml

50a62f3

Add Style/MethodCallWithArgsParentheses.

Update google-cloud-pubsub/.rubocop.yml

088ae1f

Add Style/MethodCallWithArgsParentheses.

Update google-cloud-resource_manager/.rubocop.yml

e685c8c

Add Style/MethodCallWithArgsParentheses.

Update google-cloud-spanner/.rubocop.yml

ab2aa30

Add Style/MethodCallWithArgsParentheses.

Update google-cloud-storage/.rubocop.yml

f27c617

Add Style/MethodCallWithArgsParentheses.

quartzmo removed the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Feb 15, 2019

blowmage suggested changes Feb 15, 2019

View reviewed changes

google-cloud-bigquery/lib/google/cloud/bigquery/dataset.rb Outdated Show resolved Hide resolved

google-cloud-bigquery/lib/google/cloud/bigquery/dataset.rb Outdated Show resolved Hide resolved

Update Dataset to use Bigquery::Convert.millis_to_time

50bc3fc

blowmage approved these changes Feb 15, 2019

View reviewed changes

quartzmo merged commit 736989e into googleapis:master Feb 15, 2019

quartzmo deleted the rubocop-parens branch February 15, 2019 22:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enable Rubocop cop to omit parentheses from one-line method calls #2778

Enable Rubocop cop to omit parentheses from one-line method calls #2778

quartzmo commented Jan 7, 2019

blowmage commented Jan 7, 2019

quartzmo commented Jan 7, 2019

TheRoyalTnetennba left a comment

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

quartzmo commented Jan 21, 2019

dazuma commented Jan 22, 2019

quartzmo commented Jan 22, 2019

quartzmo commented Jan 22, 2019

dazuma commented Jan 22, 2019

dazuma commented Jan 22, 2019

quartzmo commented Jan 23, 2019

dazuma commented Jan 23, 2019

quartzmo commented Jan 23, 2019

blowmage commented Jan 23, 2019

quartzmo commented Feb 8, 2019

quartzmo commented Feb 13, 2019

quartzmo commented Feb 14, 2019

Enable Rubocop cop to omit parentheses from one-line method calls #2778

Enable Rubocop cop to omit parentheses from one-line method calls #2778

Conversation

quartzmo commented Jan 7, 2019

blowmage commented Jan 7, 2019

quartzmo commented Jan 7, 2019

TheRoyalTnetennba left a comment

Choose a reason for hiding this comment

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

quartzmo commented Jan 21, 2019

dazuma commented Jan 22, 2019

quartzmo commented Jan 22, 2019

quartzmo commented Jan 22, 2019

dazuma commented Jan 22, 2019

dazuma commented Jan 22, 2019

quartzmo commented Jan 23, 2019

dazuma commented Jan 23, 2019

quartzmo commented Jan 23, 2019

blowmage commented Jan 23, 2019

quartzmo commented Feb 8, 2019

quartzmo commented Feb 13, 2019

quartzmo commented Feb 14, 2019