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

Fixes #27043 - Make rubocop work again #426

Merged
merged 8 commits into from
Aug 7, 2020

Conversation

adamruzicka
Copy link
Contributor

The behavior of Include changed in rubocop 0.56.0[1] and it started
overriding defaults provided by rubocop completely. This lead to only
rabl and rake files being checked by rubocop.

All of the file types which were manually included are being checked by
rubocop by default now and there's no need to manually include them.

[1] - rubocop/rubocop#5882

@adamruzicka
Copy link
Contributor Author

adamruzicka commented Jun 13, 2019

Test logs should say rubocop checked 182 181 files, not just 14 141.

@tbrisker
Copy link
Member

Inspecting 181 files
.......................................................................C.........C...................................................................................................

Offenses:

/home/jenkins/workspace/foreman-tasks-pull-request/database/mysql/label/fast/ruby/2.5/plugin/db/migrate/20150817102538_add_delay_attributes.rb:3:5: C: Rails/BulkChangeTable: You can use change_table :foreman_tasks_tasks, bulk: true to combine alter queries.
    add_column :foreman_tasks_tasks, :start_at, :datetime, index: true, default: nil, null: true
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
/home/jenkins/workspace/foreman-tasks-pull-request/database/mysql/label/fast/ruby/2.5/plugin/db/migrate/20180216092715_use_uuid.rb:7:7: C: Rails/BulkChangeTable: You can combine alter queries using bulk: true options.
      change_table :foreman_tasks_tasks do |t|
      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
/home/jenkins/workspace/foreman-tasks-pull-request/database/mysql/label/fast/ruby/2.5/plugin/db/migrate/20180216092715_use_uuid.rb:24:7: C: Rails/BulkChangeTable: You can combine alter queries using bulk: true options.
      change_table :foreman_tasks_tasks do |t|
      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

@adamruzicka
Copy link
Contributor Author

[test foreman-tasks]

This is odd, I'm failing to reproduce it locally and it seems unlikely that the same version of rubocop on the same version of ruby but with different db would rate the same code differently.

@ezr-ondrej
Copy link
Member

[test foreman-tasks]

@mmoll
Copy link
Contributor

mmoll commented Sep 27, 2019

this needs a rebase

The behavior of include changed in rubocop 0.56.0[1] and it started
overriding defaults provided by rubocop completely. This lead to only
rabl and rake files being checked by rubocop.

All of the file types which were manually included are being checked by
rubocop by default now and there's no need to manually include them.

[1] - rubocop/rubocop#5882
@adamruzicka
Copy link
Contributor Author

Rubocop is now green and checks 197 files

@ezr-ondrej
Copy link
Member

Could you please add following to match forman core?

Style/TrailingCommaInArrayLiteral:
  EnforcedStyleForMultiline: comma

Style/TrailingCommaInHashLiteral:
  EnforcedStyleForMultiline: comma

Style/TrailingCommaInArrayLiteral
Style/TrailingCommaInHashLiteral
@adamruzicka
Copy link
Contributor Author

Added

Copy link
Member

@ezr-ondrej ezr-ondrej left a comment

Choose a reason for hiding this comment

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

Thanks @adamruzicka ! 👍

@adamruzicka adamruzicka merged commit 2434b60 into theforeman:master Aug 7, 2020
@adamruzicka adamruzicka deleted the fix-rubocop branch August 7, 2020 09:55
@adamruzicka
Copy link
Contributor Author

Thank you @ezr-ondrej !

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