(MAINT) Rubocop updates #27

chelnak · 2022-10-01T07:57:55Z

Prior to this PR the minimum Ruby version was set to 2.0.

This PR bumps the minimum requirement to 2.7 and also adds gems required for Rubocop.

In addition it also contains fixes for files in lib and spec.

Rubocop configuration was taken from another vox project with the addition of SuggestExtensions: false.

codecov · 2022-10-01T07:58:55Z

Codecov Report

Base: 100.00% // Head: 100.00% // No change to project coverage 👍

Coverage data is based on head (0e1e147) compared to base (19ee9a9).
Patch coverage: 100.00% of modified lines in pull request are covered.

❗ Current head 0e1e147 differs from pull request most recent head 9399d7c. Consider uploading reports for the commit 9399d7c to get more accurate results

Additional details and impacted files

@@            Coverage Diff            @@
##            master       #27   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            1         1           
  Lines           61        24   -37     
=========================================
- Hits            61        24   -37

Impacted Files	Coverage Δ
lib/puppet-lint/plugins/check_trailing_comma.rb	`100.00% <100.00%> (ø)`

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

ekohl

I'll note we have https://github.com/voxpupuli/puppet-lint_modulesync_configs

I'd also prefer a step where we run Rubocop first and then the regular test matrix.

Gemfile

ekohl · 2022-10-01T11:18:57Z

.rubocop.yml

+  - "**/Puppetfile"
+  - "**/Vagrantfile"
+  - "**/Guardfile"
+Layout/LineLength:


I'm not a fan of this HUGE config. Any thoughts about reusing it like we did with our Puppet modules?

For reference: https://github.com/voxpupuli/modulesync_config/blob/f14a18d2ae681a4c973b3e898ab16e384046ca45/moduleroot/.rubocop.yml.erb#L8-L9

This means we can put all the rules in puppet-lint.gem and use it in every plugin. Note that you can still override everything in gems, you just set a baseline.

I like this idea a lot. If i'm honest, I didn't even realise it was a possibility.

I guess we could sync the merge of this PR with a new release of puppet-lint.

chelnak · 2022-10-01T13:58:41Z

Oh nice I'll take a look at these configs.

And yes makes sense to have a rubocop job, I'll make the test matrix dependent on success.

chelnak · 2022-10-02T07:37:07Z

I'm at odds with the 2.5 bundler step right now.

The test steps don't need Rubocop so we could add a bundle_without: development but the wording of that doesn't seem right.

Maybe I should rename the group to something like check?

It's early so I could be missing something obvious here!!

puppet-lint-trailing_comma-check.gemspec

Prior to this commit the minimum Ruby version was set to 2.0. This commit bumps the minimum requirement to 2.7 and also adds gems required for rubocop.

This commit adds Rubocop configuration with the TargetRubyVersion set to 2.5.

This commit removes the following ruby versions from the test matrix: * v2.4 * v2.5 * v2.6

chelnak · 2022-10-03T15:02:44Z

@ekohl @bastelfreak I'm working towards a release for puppet-lint which will include the Rubocop rules in this PR.

I'm not 100% sure on the timeline though. Could this PR go in as it is? We can inherit puppet-lints rules once it is released.

chelnak · 2022-10-03T15:04:55Z

on second thoughts it might be wise to cut the 0.4.3 release from what we already have in master as I guess this PR would be a breaking change. People may want to benefit from the array fix without having to bump to a major release.

chelnak · 2022-10-13T10:22:01Z

@ekohl I've raised this PR that adds the rubocop config to puppet-lint

puppetlabs/puppet-lint#60

I'm really hoping to get 3.0 released today!

This commit sets the minium required version of puppet-lint to v3.0.0. As of this version puppet-lints rubocop config is bundled with the gem. This means that we can now consume it in plugin projects.

This commit ensures that we inherit the rubocop configuration from puppet-lint .

chelnak · 2022-10-13T16:15:22Z

eesh I bet that failure is because I'm trying to inherit .rubocop.yml. Wasn't there an issue that prevented this?

lib/puppet-lint/plugins/check_trailing_comma.rb

ekohl · 2022-10-13T17:00:33Z

eesh I bet that failure is because I'm trying to inherit .rubocop.yml. Wasn't there an issue that prevented this?

I'm not so sure actually. Locally I don't get that error so I can't really explain it.

chelnak · 2022-10-13T17:07:49Z

I'll dig in to it a bit more later/tomorrow but it appears that the excludes are not being inherited.

FWIW I don't get it locally either .

chelnak · 2022-10-14T11:17:59Z

@ekohl I managed to replicate locally this morning:

bundle install --path vendor/gems
bundle exec rubocop

Adding the exclude for vendor/**/* in this repo allows rubocop to pass.

Another interesting observation is that .vendor/gems is in the parents excludes too but bundling in to this path locally doesn't cause the error.

I'll continue to investigate, however I've been asked to look at something else today so i'll probably get back to this on Monday.

When inheriting from a gem, the AllCops/exclude property doesn't seem to be propagated. Running rubocop with --degbug confirms that it is recognising the directive to inherit .rubocop.yml from puppet-line and rules seem to be valid. This commit adds the AllCops/Exclude containing a single value that excludes the "vendor" directory that appears when running in ci.

chelnak · 2022-10-19T14:01:01Z

@ekohl

Had another look at this. If you run rubocop with --debug, you can see that the settings are being properly inherited from puppet-lint. However, it still seems to disregard AllCops/Exclude.

I've added a new commit that adds the excludes to the local rubocop file.

I wonder if this is an appropriate workaround?

ekohl · 2022-10-20T11:49:13Z

Could it be that this ancient version of Rubocop doesn't properly support it?

lib/puppet-lint/plugins/check_trailing_comma.rb

chelnak · 2022-10-20T21:02:50Z

Could it be that this ancient version of Rubocop doesn't properly support it?

Turns out the answer to this is no (actually to my surprise)!

It seems to be some sort of conflict inheriting a config file called .rubocop.yml.

Renaming to something arbitrary and appears to work.

chelnak · 2022-10-28T12:39:59Z

@ekohl

Pondering on this topic a bit..

How would you feel about referencing the baseline config from a url rather than the gem? It would mean we are not tied in to gem releases to get config updates.

https://docs.rubocop.org/rubocop/configuration.html#inheriting-configuration-from-a-remote-url

ekohl · 2022-10-28T12:42:58Z

How would you feel about referencing the baseline config from a url rather than the gem?

This essentially means we can't do versioning. Let's say a new major version comes out: do you version the URL? Then what's the benefit compared to using it from the gem? It also makes it harder to test out new changes IMHO. Now you can change your Gemfile to point to a local checkout (gem 'puppet-lint', path: '../puppet-lint') and test out changes. You'd lose that with URLs. So IMHO it'd be a downside.

ekohl · 2022-10-28T13:00:22Z

Btw, I'd also consider a puppet-lint-rubocop package that has exact dependencies on Rubocop versions and the config. Then puppet-lint itself and all plugins can depend on that. It'd remove the need to sync the versions.

chelnak · 2022-10-29T13:36:21Z

It's interesting that you mention the rubocop gem as it's something I'd thought about for the modules.

My main issue is that I keep coming across dependency conflicts which slows this sort of progression.. however it would be cool to isolate as much as possible..

ekohl · 2022-10-29T17:59:56Z

It's interesting that you mention the rubocop gem as it's something I'd thought about for the modules.

For Vox Pupuli we already use this: https://github.com/voxpupuli/voxpupuli-test/blob/master/rubocop.yml

chelnak marked this pull request as draft October 1, 2022 08:02

ekohl requested changes Oct 1, 2022

View reviewed changes

chelnak force-pushed the maint-rubocop_updates branch 3 times, most recently from 196c32b to 7ad68f6 Compare October 2, 2022 07:04

chelnak marked this pull request as ready for review October 2, 2022 07:04

chelnak force-pushed the maint-rubocop_updates branch 5 times, most recently from f7321a4 to c5018d6 Compare October 2, 2022 07:32

bastelfreak reviewed Oct 2, 2022

View reviewed changes

puppet-lint-trailing_comma-check.gemspec Outdated Show resolved Hide resolved

chelnak force-pushed the maint-rubocop_updates branch 2 times, most recently from 9d658e2 to 39c3b20 Compare October 3, 2022 14:58

chelnak added 5 commits October 3, 2022 15:58

(MAINT) Bump ruby version and add Rubocop

0c0c32d

Prior to this commit the minimum Ruby version was set to 2.0. This commit bumps the minimum requirement to 2.7 and also adds gems required for rubocop.

(MAINT) Add Rubocop config

1145244

This commit adds Rubocop configuration with the TargetRubyVersion set to 2.5.

(MAINT) Rubocop - fixes for files in lib

682c2ce

(MAINT) Rubocop - fixes for files in spec

e940ab1

(MAINT) Remove unsupported rubies test workflow

65a032c

This commit removes the following ruby versions from the test matrix: * v2.4 * v2.5 * v2.6

chelnak force-pushed the maint-rubocop_updates branch from 39c3b20 to 65a032c Compare October 3, 2022 14:58

chelnak requested review from ekohl and bastelfreak October 3, 2022 15:00

chelnak added 2 commits October 13, 2022 16:57

(MAINT) Bump puppet-lint dependency

5be3193

This commit sets the minium required version of puppet-lint to v3.0.0. As of this version puppet-lints rubocop config is bundled with the gem. This means that we can now consume it in plugin projects.

(MAINT) Inherit rubocop config from puppet-lint

07ae714

This commit ensures that we inherit the rubocop configuration from puppet-lint .

ekohl reviewed Oct 13, 2022

View reviewed changes

lib/puppet-lint/plugins/check_trailing_comma.rb Show resolved Hide resolved

chelnak force-pushed the maint-rubocop_updates branch from ebb94fc to ea1b036 Compare October 20, 2022 19:52

ekohl reviewed Oct 20, 2022

View reviewed changes

lib/puppet-lint/plugins/check_trailing_comma.rb Show resolved Hide resolved

chelnak force-pushed the maint-rubocop_updates branch from ea1b036 to 9399d7c Compare October 20, 2022 20:02

chelnak self-assigned this Oct 28, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

(MAINT) Rubocop updates #27

(MAINT) Rubocop updates #27

chelnak commented Oct 1, 2022 •

edited

Loading

codecov bot commented Oct 1, 2022 •

edited

Loading

ekohl left a comment

ekohl Oct 1, 2022

chelnak Oct 2, 2022

chelnak commented Oct 1, 2022

chelnak commented Oct 2, 2022

chelnak commented Oct 3, 2022

chelnak commented Oct 3, 2022

chelnak commented Oct 13, 2022

chelnak commented Oct 13, 2022

ekohl commented Oct 13, 2022

chelnak commented Oct 13, 2022

chelnak commented Oct 14, 2022

chelnak commented Oct 19, 2022

ekohl commented Oct 20, 2022

chelnak commented Oct 20, 2022

chelnak commented Oct 28, 2022

ekohl commented Oct 28, 2022

ekohl commented Oct 28, 2022

chelnak commented Oct 29, 2022

ekohl commented Oct 29, 2022

(MAINT) Rubocop updates #27

Are you sure you want to change the base?

(MAINT) Rubocop updates #27

Conversation

chelnak commented Oct 1, 2022 • edited Loading

codecov bot commented Oct 1, 2022 • edited Loading

Codecov Report

ekohl left a comment

Choose a reason for hiding this comment

ekohl Oct 1, 2022

Choose a reason for hiding this comment

chelnak Oct 2, 2022

Choose a reason for hiding this comment

chelnak commented Oct 1, 2022

chelnak commented Oct 2, 2022

chelnak commented Oct 3, 2022

chelnak commented Oct 3, 2022

chelnak commented Oct 13, 2022

chelnak commented Oct 13, 2022

ekohl commented Oct 13, 2022

chelnak commented Oct 13, 2022

chelnak commented Oct 14, 2022

chelnak commented Oct 19, 2022

ekohl commented Oct 20, 2022

chelnak commented Oct 20, 2022

chelnak commented Oct 28, 2022

ekohl commented Oct 28, 2022

ekohl commented Oct 28, 2022

chelnak commented Oct 29, 2022

ekohl commented Oct 29, 2022

chelnak commented Oct 1, 2022 •

edited

Loading

codecov bot commented Oct 1, 2022 •

edited

Loading