Go Maeda, do you see any downside for having Rubocop as part of the core?
--------------------------------------------------------------------------------
Marius BALTEANU wrote:
> Go Maeda, do you see any downside for having Rubocop as part of the core?

I am in favor of adding RuboCop. Actually, I sometimes check patches with RuboCop on my dev environment.

But I think @TargetRubyVersion@ in the suggested patch should be @2.3@ instead of @2.5@ because the oldest Ruby version Redmine supports is 2.3.
--------------------------------------------------------------------------------

--------------------------------------------------------------------------------
Go MAEDA wrote:
> I am in favor of adding RuboCop. Actually, I sometimes check patches with RuboCop on my dev environment.
>
> But I think @TargetRubyVersion@ in the suggested patch should be @2.3@ instead of @2.5@ because the oldest Ruby version Redmine supports is 2.3.

Great, thanks for your quick reply. Here is the updated patch.
--------------------------------------------------------------------------------
I propose adding .rubocop_todo.yml and removing "DisabledByDefault: true".

With the original patch, RuboCop performs only a few checks. But I want to perform various checks with RuboCop when I write or review patches.
--------------------------------------------------------------------------------
Go MAEDA wrote:
> I propose adding .rubocop_todo.yml and removing "DisabledByDefault: true".

I like your proposal, but I think we should apply the following changes on top of your patch:

<pre><code class="diff">
vagrant@jessie:/vagrant/project/redmine$ git diff
diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml
index 16a8d62..1cdb965 100644
--- a/.rubocop_todo.yml
+++ b/.rubocop_todo.yml
@@ -491,6 +491,7 @@ Layout/SpaceAroundOperators:
# SupportedStylesForEmptyBraces: space, no_space
Layout/SpaceBeforeBlockBraces:
EnforcedStyleForEmptyBraces: no_space
+ Enabled: false

# Offense count: 7
# Cop supports --auto-correct.
@@ -1302,6 +1303,12 @@ Rails/TimeZone:
Rails/Validation:
Enabled: false

+# Offense count: 3
+Rails/BulkChangeTable:
+ Exclude:
+ - 'db/migrate/20120714122200_add_workflows_rule_fields.rb'
+ - 'db/migrate/20131214094309_remove_custom_fields_min_max_length_default_values.rb'
+
# Offense count: 4
Security/Eval:
Exclude:
</code></pre>

Otherwise, we'll have the following fails: https://gitlab.com/redmine-org/redmine/-/jobs/229555011

--------------------------------------------------------------------------------

--------------------------------------------------------------------------------
+1
--------------------------------------------------------------------------------
I have updated the patch:

* Added rules that Marius pointed out in #31509#note-6. I have added rules to @.rubocop.yml@ instead of updating @.rubocop_todo.yml@ because @.rubocop_todo.yml@ is an auto-generated file and it will be overwritten in the future
* Removed @Layout/TrailingWhitespace@. Since all checks are enabled now, we don't have to explicitly enable it
* Added @Style/HashSyntax@ to allow Ruby 1.8 style hash syntax (@{:foo => 1, :bar => 2}@)
* Updated doc/RUNNING_TESTS

--------------------------------------------------------------------------------
Go MAEDA wrote:
> I have updated the patch:
>
> * Added rules that Marius pointed out in #31509#note-6. I have added rules to @.rubocop.yml@ instead of updating @.rubocop_todo.yml@ because @.rubocop_todo.yml@ is an auto-generated file and it will be overwritten in the future
> * Removed @Layout/TrailingWhitespace@. Since all checks are enabled now, we don't have to explicitly enable it
> * Added @Style/HashSyntax@ to allow Ruby 1.8 style hash syntax (@{:foo => 1, :bar => 2}@)
> * Updated doc/RUNNING_TESTS

Looks very good to me, thank you for improving my patch!

--------------------------------------------------------------------------------
Committed. Thank you for your contribution.
--------------------------------------------------------------------------------
RuboCop 0.72.0 has been released on July 25th.
https://rubygems.org/gems/rubocop/versions/0.72.0

Here is a patch to update RuboCop to 0.72.0.
--------------------------------------------------------------------------------
Go MAEDA wrote:
> RuboCop 0.72.0 has been released on July 25th.
> https://rubygems.org/gems/rubocop/versions/0.72.0
>
> Here is a patch to update RuboCop to 0.72.0.

Committed the patch in r18320.

--------------------------------------------------------------------------------