Support Puppet >= 4.10 #50
Conversation
|
Rubocop seems to become more picky than in the past. I see 40 offenses within the existing code and this change is not even touching ruby code. |
Remove dependency on 'pe' as it will fail in any metadata-json-lint tests.
| Bundler/DuplicatedGem: | ||
| Exclude: | ||
| - "Gemfile" | ||
|
|
There was a problem hiding this comment.
see rubocop/rubocop#3752 if interessted in more details
There was a problem hiding this comment.
Wow, what a mess. Thanks for figuring this out.
| Metrics/BlockLength: | ||
| Exclude: | ||
| - "spec/**/*.rb" | ||
|
|
There was a problem hiding this comment.
see rubocop/rubocop#3772 if interessted in more details
There was a problem hiding this comment.
Why disable entirely on spec tests? Is the idea that we just use it for actual ruby code in lib/ ?
There was a problem hiding this comment.
This cop get's angry about code blocks with more than 25 lines. It would need a complete refactor to satisfy him. The longest block is 1487 lines at the moment.
I think this cop is good for real ruby code but not needed on the spec tests itself.
| # This cop isn't compatible with Ruby < 2.0 | ||
| Style/SymbolArray: | ||
| Exclude: | ||
| - "Rakefile" |
There was a problem hiding this comment.
There was a problem hiding this comment.
Why ignore the Rakefile? I thought that since it is ruby, it should be tested. I'm not sure what I was supposed to get from looking at that Travis link.
There was a problem hiding this comment.
This cop enforces to use a syntax that is not available on Ruby < 2.0.
You can see the actual error message in one of the failed tests [1].
[1] https://travis-ci.org/ghoneycutt/puppet-module-nscd/jobs/220867193#L213-L216
|
Yes, rubocop is not very stable. I'm all for standardized code but not at the cost of just continually churning it around. I'm happy to add a bunch of ignore/exclude lines :) |
| @@ -740,7 +740,7 @@ | |||
| end | |||
|
|
|||
| context 'as an array' do | |||
| let(:params) { { :package_name => %w(nscd foo) } } | |||
| let(:params) { { :package_name => %w[nscd foo] } } | |||
There was a problem hiding this comment.
this seems like rubocop nonsense
There was a problem hiding this comment.
If you don't like that, I can turn of the responding cop too.
There was a problem hiding this comment.
Please do turn off the cop that wants to change all the %w() to %w[]. This way we reduce code churn and make other PR's easier to merge. Funny thing is we went to %w() because of rubocop :>
Other than that, everything looks good! Thanks!
|
@ghoneycutt please keep in mind that I only have disabled specific cops for specific files. This is the minimally invasive configuration change I could think of. |
| @@ -740,7 +740,7 @@ | |||
| end | |||
|
|
|||
| context 'as an array' do | |||
| let(:params) { { :package_name => %w(nscd foo) } } | |||
| let(:params) { { :package_name => %w[nscd foo] } } | |||
There was a problem hiding this comment.
Please do turn off the cop that wants to change all the %w() to %w[]. This way we reduce code churn and make other PR's easier to merge. Funny thing is we went to %w() because of rubocop :>
Other than that, everything looks good! Thanks!
|
Released in v1.10.0 |
No description provided.