voxpupuli / metadata-json-lint

Tool to check the validity of Puppet metadata.json files
Apache License 2.0
29 stars 27 forks source link

Allow setting options for the rake task or custom rake tasks #50

Closed vStone closed 7 years ago

vStone commented 7 years ago

This could be a solution to #18

ghoneycutt commented 7 years ago

Thanks @vStone - Looking forward to this feature. Could you please add some tests to show that this works.

vStone commented 7 years ago

I attempted to write rspec tests for rake tasks but I was in over my head. In stead I modified test.sh to split out bin and rake tasks.

vStone commented 7 years ago

I find it a lot more elegant in usage from a rake task. Its cosmetics: you can use either [hash] stuff or .accesors with protection from typos in custom rake tasks

require 'metadata-json-lint'
task :metadata_lint do
  MetadataJsonLint.parse('metadata.json') do |options|
      options.strict_license = false
      options.fail_on_warnings = false
  end
end

vs

require 'metadata-json-lint'
task :metadata_lint do
  MetadataJsonLint.parse('metadata.json') do |options|
      options[:strict_license] = false
      options[:fail_on_warnings] = false
  end
end

IMHO using a hash gives the end-user the illusion the options behaves like a hash: you can just add any value to it. On the other hand, if the switch to a struct is 'blocking' for getting this merged, I'll gladly change it back to a simple hash.

rnelson0 commented 7 years ago

@vStone It's not blocking at all. I don't like to prematurely optimize for a future where we might have fleventyleven options, I just needed to understand the benefit. We can always revisit it if we get to that future.

However, the tests are failing, looks like we'll need a rubocop check turned off on the module so that tests go green.