Fix Rubocop rules #182
Fix Rubocop rules #182
Conversation
|
|
||
| Layout/ExtraSpacing: | ||
| Enabled: true | ||
|
|
||
| # JobTemplates use this for inputs, we should probably whitelist classes | ||
| Security/YAMLLoad: |
There was a problem hiding this comment.
This will be an issue with Ruby 3 where load became safe_load.
There was a problem hiding this comment.
@ekohl what is this about? could you please brief more.
There was a problem hiding this comment.
https://docs.rubocop.org/rubocop/cops_security.html#securityyamlload already has a note. Looks like it's Ruby 3.1, not 3.0.
| @@ -2,7 +2,7 @@ module ForemanTemplates | |||
| class ExportResult | |||
| attr_reader :template, :name, :template_file, :exported, :additional_info | |||
|
|
|||
| def initialize(template, exported = true) | |||
| def initialize(template, exported: true) | |||
There was a problem hiding this comment.
I think either this API change breaks things, or it's the one parse_result:
Failure:
test_0008_should force update locked template on import(Minitest::Result) [/home/jenkins/workspace/foreman_templates-pull-request/label/fast/ruby/2.7/plugin/test/functional/api/v2/template_controller_test.rb:110]:
Expected response to be a <2XX: success>, but was a <500: Internal Server Error>
Response body: {
"error": {"message":"wrong number of arguments (given 1, expected 0)"}
}
But the commit Fix Style/OptionalBooleanParameter cop breaks things. It's not a safe cop where you can simply change things, just like I said in theforeman/foreman-installer#883 (comment).
There was a problem hiding this comment.
Even though boolean kw parameter looks nicer, I'd not change signatures just now to make rubocop happy.
| @@ -2,7 +2,7 @@ module ForemanTemplates | |||
| class ExportResult | |||
| attr_reader :template, :name, :template_file, :exported, :additional_info | |||
|
|
|||
| def initialize(template, exported = true) | |||
| def initialize(template, exported: true) | |||
There was a problem hiding this comment.
Even though boolean kw parameter looks nicer, I'd not change signatures just now to make rubocop happy.
| @@ -7,7 +7,7 @@ def initialize(template_file) | |||
| @template_file = template_file.split('/').last | |||
| end | |||
|
|
|||
| def to_h(verbose = false) | |||
There was a problem hiding this comment.
Same here, let's stick with the old optional argument.
|
@archanaserver, could you please rebase? |
archanaserver
force-pushed
the
rubocop
branch
from
a363aab to
58f8746
Compare
archanaserver
force-pushed
the
rubocop
branch
2 times, most recently
from
e3a8c97 to
27c9f28
Compare
There was a problem hiding this comment.
Well, after rebasing it seems there is nothing left to fix. Maybe I'd treat this PR to ensure we resolved the issue Ewoud mentioned in https://github.com/theforeman/foreman_templates/pull/182/files#r1467571359
| @@ -26,7 +26,7 @@ class TemplateControllerTest < ::ActionController::TestCase | |||
| FactoryBot.create(:provisioning_template, :name => 'export_test_template') | |||
| post :export, params: { "repo" => tmpdir, "filter" => "^export_test_template", "negate" => true, "metadata_export_mode" => "keep" } | |||
| assert_response :success | |||
| refute_includes(Dir.entries("#{tmpdir}/provisioning_templates/provision"), 'export_test_template.erb') | |||
There was a problem hiding this comment.
I guess, I've skipped this while rebasing, lemme fix it!
archanaserver
force-pushed
the
rubocop
branch
from
27c9f28 to
283964b
Compare
#180