Use latest rubocop and target ruby 2.5 #40

alexjfisher · 2021-03-13T17:53:53Z

No description provided.

smortex · 2021-04-29T18:52:02Z

Since Puppetlabs only support Puppet 6+ now, I think we can even consider jumping to targeting Ruby 2.5 which is the version Puppet 6 use and has reached EOL only a few weeks ago?

ekohl · 2021-05-23T13:08:36Z

I'm starting to add Rubocop to theforeman modules and I'm running into this as well now.

bastelfreak · 2021-05-23T14:51:19Z

I agree, I think we should move forward with it

ekohl · 2021-06-21T13:48:27Z

Related Github formatter introduced in Rubocop 1.2: puppetlabs/puppetlabs_spec_helper#340

ekohl · 2021-07-26T09:31:10Z

puppetlabs/puppetlabs_spec_helper#340 has been merged. Once there's a release out, it'd be great if this PR also raised the minimum spec helper version to that.

smortex · 2021-08-11T17:37:00Z

puppetlabs/puppetlabs_spec_helper#340 has been merged. Once there's a release out, it'd be great if this PR also raised the minimum spec helper version to that.

This seems to have been released just after your message:
https://github.com/puppetlabs/puppetlabs_spec_helper/releases/tag/v4.0.0

traylenator · 2021-10-01T08:47:09Z

Some extra motivation Puppetlabs now expect rubocop-performance to be present which needs newer rubocop.

e.g. puppetlabs-inifile results in

rubocop (1.6.1)
rubocop-ast (1.12.0)
rubocop-performance (1.9.1)
rubocop-rspec (2.0.1)

traylenator · 2021-10-07T12:24:33Z

Okay so with extra commit on top for even latest rubocop again and targeting ruby 2.5 traylenator@20b292c

Ignored modules

Ignore puppet-openssl as Gemfile did not work.
Ignore puppet-archive, puppet-epel and puppet-jenkins as failing with 0.49.1 already.

Invalid config in module

puppet-vault_lookup

.rubocop.yml: Metrics/LineLength has the wrong namespace - should be Layout
Error: The `Layout/IndentHeredoc` cop has been renamed to `Layout/HeredocIndentation`.
(obsolete configuration found in .rubocop.yml, please update it)

Per module counts

e.g

puppet-allknowingdns 6 files inspected, 32 offenses detected, 17 offenses auto-correctable
puppet-alternatives 10 files inspected, 53 offenses detected, 47 offenses auto-correctable
...
count.txt

Count of un-Correctable Critical problems

e.g.

2421 RSpec/ContextWording: Start context description with 'when', 'with', or 'without'.
544 Naming/HeredocDelimiterNaming: Use meaningful heredoc delimiters.
....

unfixable.txt

Count of un-Correctable Critical by module

e.g.

puppet-allknowingdns_rubocop_result.txt
8 RSpec/ContextWording: Start context description with 'when', 'with', or 'without'.
3 Style/MixinUsage: include is used at the top level. Use inside class or module.
1 RSpec/RepeatedExampleGroupDescription: Repeated describe block description on line(s) [19]
1 RSpec/RepeatedExampleGroupDescription: Repeated describe block description on line(s) [14]
1 RSpec/RepeatedExampleGroupBody: Repeated describe block body on line(s) [19]
1 RSpec/RepeatedExampleGroupBody: Repeated describe block body on line(s) [14]

unfixable-bymodule.txt

genebean · 2021-10-15T13:27:53Z

It is probably worth updating the PR title to match how this has evolved too.

smortex · 2021-10-16T00:50:47Z

@traylenator looks like a move in the right direction. Do you mind adding your changes to this PR?

Naming/HeredocDelimiterNaming and RSpec/* are the most frequent offenses. Maybe we can ignore them if this is a concern?

traylenator · 2021-10-18T07:50:39Z

@smortex unsure how to update this PR with my changes - short of creating a whole new PR ?

alexjfisher · 2021-10-18T08:50:39Z

@traylenator Are you able to push to this PR now?

codecov · 2021-10-18T09:01:25Z

Codecov Report

Merging #40 (7c0ad20) into master (cfe8c99) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master      #40   +/-   ##
=======================================
  Coverage   28.33%   28.33%           
=======================================
  Files           2        2           
  Lines          60       60           
=======================================
  Hits           17       17           
  Misses         43       43

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cfe8c99...7c0ad20. Read the comment docs.

voxpupuli-test.gemspec

.github/workflows/test.yml

bastelfreak · 2021-10-18T09:15:54Z

rubocop.yml

-  TargetRubyVersion: 1.9
+  # Ruby 2.4 is the oldest version we can use with the latest version of rubocop (1.11.0 as of 13/03/2021).
+  # Puppet agent 5 ships with ruby 2.4.  Puppetserver 5 has JRuby 2.1.x which is only ruby 2.3 compatible, but this
+  # is unlikely to cause many issues.  If/when it does, we can disable the relevant cop inline as appropriate.


maybe add a note that we chose Ruby 2.5 because that's in Puppet Agent 6 vendored and that's the oldest agent we still support.

Out of date docs updated.

ekohl · 2021-10-18T10:42:51Z

2421 RSpec/ContextWording: Start context description with 'when', 'with', or 'without'.

544 Naming/HeredocDelimiterNaming: Use meaningful heredoc delimiters.

These are very high. Should we ignore these?

alexjfisher · 2021-10-18T11:05:09Z

2421 RSpec/ContextWording: Start context description with 'when', 'with', or 'without'.

544 Naming/HeredocDelimiterNaming: Use meaningful heredoc delimiters.

These are very high. Should we ignore these?

I'm happy to create todo files so that we don't disable the cops for all future code too.

alexjfisher · 2021-10-18T11:05:52Z

2421 RSpec/ContextWording: Start context description with 'when', 'with', or 'without'.

544 Naming/HeredocDelimiterNaming: Use meaningful heredoc delimiters.

These are very high. Should we ignore these?

I'm happy to create todo files so that we don't disable the cops for all future code too.

Actually, I'm happy either way! :)

ekohl · 2021-10-19T09:46:40Z

Given it's test code and many Puppet developers are not Ruby developers, I'm leaning to disabling the cops. IMHO we're not going for perfect code quality so let's not set the bar too high.

alexjfisher · 2021-10-19T12:57:51Z

Given it's test code and many Puppet developers are not Ruby developers, I'm leaning to disabling the cops. IMHO we're not going for perfect code quality so let's not set the bar too high.

Updated. (but not tested!)

smortex

Updated. (but not tested!)

217 -> 123 offenses detected on WIP puppet-elasticsearch

smortex · 2021-10-25T22:42:54Z

FWIW, with more testing I am experiencing a lot of these:

spec/spec_utilities.rb:112:8: C: [Corrected] Layout/DotPosition: Place the . on the previous line, together with the method call receiver.
    end.empty?
       ^

89 files inspected, 38 offenses detected, 3 offenses corrected
Infinite loop detected in /usr/home/romain/Projects/voxpupuli/modulesync_config/modules/voxpupuli/puppet-elasticsearch/spec/spec_utilities.rb and caused by Layout/DotPosition
/usr/home/romain/.vendor/ruby/2.7/gems/rubocop-1.22.2/lib/rubocop/runner.rb:298:in `check_for_infinite_loop'
/usr/home/romain/.vendor/ruby/2.7/gems/rubocop-1.22.2/lib/rubocop/runner.rb:281:in `block in iterate_until_no_changes'
/usr/home/romain/.vendor/ruby/2.7/gems/rubocop-1.22.2/lib/rubocop/runner.rb:280:in `loop'
/usr/home/romain/.vendor/ruby/2.7/gems/rubocop-1.22.2/lib/rubocop/runner.rb:280:in `iterate_until_no_changes'
/usr/home/romain/.vendor/ruby/2.7/gems/rubocop-1.22.2/lib/rubocop/runner.rb:249:in `do_inspection_loop'
/usr/home/romain/.vendor/ruby/2.7/gems/rubocop-1.22.2/lib/rubocop/runner.rb:130:in `block in file_offenses'
/usr/home/romain/.vendor/ruby/2.7/gems/rubocop-1.22.2/lib/rubocop/runner.rb:155:in `file_offense_cache'
/usr/home/romain/.vendor/ruby/2.7/gems/rubocop-1.22.2/lib/rubocop/runner.rb:129:in `file_offenses'
/usr/home/romain/.vendor/ruby/2.7/gems/rubocop-1.22.2/lib/rubocop/runner.rb:120:in `process_file'
/usr/home/romain/.vendor/ruby/2.7/gems/rubocop-1.22.2/lib/rubocop/runner.rb:101:in `block in each_inspected_file'
/usr/home/romain/.vendor/ruby/2.7/gems/rubocop-1.22.2/lib/rubocop/runner.rb:100:in `each'
/usr/home/romain/.vendor/ruby/2.7/gems/rubocop-1.22.2/lib/rubocop/runner.rb:100:in `reduce'
/usr/home/romain/.vendor/ruby/2.7/gems/rubocop-1.22.2/lib/rubocop/runner.rb:100:in `each_inspected_file'
/usr/home/romain/.vendor/ruby/2.7/gems/rubocop-1.22.2/lib/rubocop/runner.rb:86:in `inspect_files'
/usr/home/romain/.vendor/ruby/2.7/gems/rubocop-1.22.2/lib/rubocop/runner.rb:47:in `run'
/usr/home/romain/.vendor/ruby/2.7/gems/rubocop-1.22.2/lib/rubocop/cli/command/execute_runner.rb:26:in `block in execute_runner'
/usr/home/romain/.vendor/ruby/2.7/gems/rubocop-1.22.2/lib/rubocop/cli/command/execute_runner.rb:52:in `with_redirect'
/usr/home/romain/.vendor/ruby/2.7/gems/rubocop-1.22.2/lib/rubocop/cli/command/execute_runner.rb:25:in `execute_runner'
/usr/home/romain/.vendor/ruby/2.7/gems/rubocop-1.22.2/lib/rubocop/cli/command/execute_runner.rb:17:in `run'
/usr/home/romain/.vendor/ruby/2.7/gems/rubocop-1.22.2/lib/rubocop/cli/command.rb:11:in `run'
/usr/home/romain/.vendor/ruby/2.7/gems/rubocop-1.22.2/lib/rubocop/cli/environment.rb:18:in `run'
/usr/home/romain/.vendor/ruby/2.7/gems/rubocop-1.22.2/lib/rubocop/cli.rb:71:in `run_command'
/usr/home/romain/.vendor/ruby/2.7/gems/rubocop-1.22.2/lib/rubocop/cli.rb:78:in `execute_runners'
/usr/home/romain/.vendor/ruby/2.7/gems/rubocop-1.22.2/lib/rubocop/cli.rb:47:in `run'
/usr/home/romain/.vendor/ruby/2.7/gems/rubocop-1.22.2/exe/rubocop:12:in `block in <top (required)>'
/usr/local/lib/ruby/2.7/benchmark.rb:308:in `realtime'
/usr/home/romain/.vendor/ruby/2.7/gems/rubocop-1.22.2/exe/rubocop:12:in `<top (required)>'
/home/romain/.vendor/ruby/2.7/bin/rubocop:23:in `load'
/home/romain/.vendor/ruby/2.7/bin/rubocop:23:in `<main>'

Autocorrect fails at fixing these, and the manual fix is not that better IMHO:

-    end.empty?
+    end.
+      empty?

Any multi-line block / hash followed by .something trigger this 😱 .

Maybe we can disable this test (Layout/DotPosition)?

alexjfisher · 2021-10-27T09:45:48Z

@smortex Rubocop bug fixed in rubocop/rubocop#10207

4.0.0 provides rubocop annotations in GitHub Actions. This version was tested in voxpupuli/puppet-wireguard#9.

It was decided that leaving these enabled word create too much unnecessary work.

bastelfreak · 2021-10-28T10:53:36Z

I will merge this now and make a new major release. If the rubocop config doesn't turn out be be useable/we experience trouble we've always the option to adjust it.

traylenator · 2021-11-08T10:34:35Z

Thanks for merging - was offline a couple of weeks.

alexjfisher added the backwards-incompatible label Mar 13, 2021

alexjfisher marked this pull request as draft March 13, 2021 17:54

alexjfisher mentioned this pull request Mar 13, 2021

WIP: Test out new rubocop config voxpupuli/puppet-extlib#177

Closed

ekohl mentioned this pull request May 23, 2021

Add a Rubocop config theforeman/puppet-dns#188

Closed

smortex changed the title ~~Use latest rubocop and target ruby 2.4~~ Use latest rubocop and target ruby 2.5 Oct 15, 2021

smortex changed the title ~~Use latest rubocop and target ruby 2.5~~ Use latest rubocop and target ruby 2.4 Oct 16, 2021

smortex mentioned this pull request Oct 16, 2021

Modulesync voxpupuli/puppet-elastic_stack#38

Closed

alexjfisher changed the title ~~Use latest rubocop and target ruby 2.4~~ Use latest rubocop and target ruby 2.5 Oct 18, 2021

Use latest rubocop and target ruby 2.4

16a6ca4

alexjfisher force-pushed the new_rubocop branch from 25de071 to 16a6ca4 Compare October 18, 2021 09:00

alexjfisher force-pushed the new_rubocop branch from 51bec0f to 15ad3aa Compare October 18, 2021 09:07

bastelfreak reviewed Oct 18, 2021

View reviewed changes

voxpupuli-test.gemspec Outdated Show resolved Hide resolved

bastelfreak reviewed Oct 18, 2021

View reviewed changes

.github/workflows/test.yml Show resolved Hide resolved

alexjfisher mentioned this pull request Oct 18, 2021

require puppetlabs_spec_helper 4 #46

Closed

bastelfreak reviewed Oct 18, 2021

View reviewed changes

alexjfisher force-pushed the new_rubocop branch from 15ad3aa to e5fc4de Compare October 18, 2021 10:23

alexjfisher marked this pull request as ready for review October 18, 2021 10:24

bastelfreak approved these changes Oct 18, 2021

View reviewed changes

smortex approved these changes Oct 18, 2021

View reviewed changes

smortex mentioned this pull request Oct 18, 2021

Modulesync voxpupuli/puppet-elastic_stack#39

Merged

smortex approved these changes Oct 19, 2021

View reviewed changes

traylenator and others added 3 commits October 28, 2021 10:18

Even newer rubocop 1.22.3 and ruby 2.5

7a1ea95

require puppetlabs_spec_helper 4

604c368

4.0.0 provides rubocop annotations in GitHub Actions. This version was tested in voxpupuli/puppet-wireguard#9.

Disable RSpec/ContextWording and Naming/HeredocDelimiterNaming

7c0ad20

It was decided that leaving these enabled word create too much unnecessary work.

alexjfisher force-pushed the new_rubocop branch from c8cb454 to 7c0ad20 Compare October 28, 2021 09:19

bastelfreak approved these changes Oct 28, 2021

View reviewed changes

bastelfreak merged commit 36c46e2 into voxpupuli:master Oct 28, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use latest rubocop and target ruby 2.5 #40

Use latest rubocop and target ruby 2.5 #40

alexjfisher commented Mar 13, 2021

smortex commented Apr 29, 2021

ekohl commented May 23, 2021

bastelfreak commented May 23, 2021

ekohl commented Jun 21, 2021

ekohl commented Jul 26, 2021

smortex commented Aug 11, 2021

traylenator commented Oct 1, 2021

traylenator commented Oct 7, 2021

genebean commented Oct 15, 2021

smortex commented Oct 16, 2021

traylenator commented Oct 18, 2021

alexjfisher commented Oct 18, 2021

codecov bot commented Oct 18, 2021 •

edited

bastelfreak Oct 18, 2021

alexjfisher Oct 18, 2021

ekohl commented Oct 18, 2021

alexjfisher commented Oct 18, 2021

alexjfisher commented Oct 18, 2021

ekohl commented Oct 19, 2021

alexjfisher commented Oct 19, 2021

smortex left a comment •

edited

smortex commented Oct 25, 2021

alexjfisher commented Oct 27, 2021

bastelfreak commented Oct 28, 2021

traylenator commented Nov 8, 2021

Use latest rubocop and target ruby 2.5 #40

Use latest rubocop and target ruby 2.5 #40

Conversation

alexjfisher commented Mar 13, 2021

smortex commented Apr 29, 2021

ekohl commented May 23, 2021

bastelfreak commented May 23, 2021

ekohl commented Jun 21, 2021

ekohl commented Jul 26, 2021

smortex commented Aug 11, 2021

traylenator commented Oct 1, 2021

traylenator commented Oct 7, 2021

Ignored modules

Invalid config in module

Per module counts

Count of un-Correctable Critical problems

Count of un-Correctable Critical by module

genebean commented Oct 15, 2021

smortex commented Oct 16, 2021

traylenator commented Oct 18, 2021

alexjfisher commented Oct 18, 2021

codecov bot commented Oct 18, 2021 • edited

Codecov Report

bastelfreak Oct 18, 2021

Choose a reason for hiding this comment

alexjfisher Oct 18, 2021

Choose a reason for hiding this comment

ekohl commented Oct 18, 2021

alexjfisher commented Oct 18, 2021

alexjfisher commented Oct 18, 2021

ekohl commented Oct 19, 2021

alexjfisher commented Oct 19, 2021

smortex left a comment • edited

Choose a reason for hiding this comment

smortex commented Oct 25, 2021

alexjfisher commented Oct 27, 2021

bastelfreak commented Oct 28, 2021

traylenator commented Nov 8, 2021

codecov bot commented Oct 18, 2021 •

edited

smortex left a comment •

edited