-
Notifications
You must be signed in to change notification settings - Fork 452
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add support for passing multiple plugin directories on CLI/client.rb #1221
Conversation
lib/ohai/system.rb
Outdated
Ohai.config[:plugin_path] << Ohai.config[:directory] | ||
# add any additional CLI passed directories to the plugin path excluding duplicates | ||
unless Ohai.config[:directory].nil? | ||
Ohai.config[:directory].each do |dir| |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should probably be Array(Ohai.config[:directory]).each
so it still supports scalars, otherwise you break people who might have been setting this value in config.
Other than @lamont-granquist 's comment, I'm 👍 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
You have some chefstyle failures to cleanup before you merge... but code looks good.
I'm not sure where those chefstyle warnings are coming from. All 3 are bogus and don't show up locally with bundle exec chefstyle. I'm on the same version too: Chefstyle 0.10.0
|
lib/ohai/application.rb
Outdated
proc: lambda { |path| Ohai::Config.platform_specific_path(path) } | ||
description: "A directory to add to the Ohai plugin search path. If passing multiple directories comma separate directories.", | ||
proc: lambda { |paths| | ||
paths = paths.split(",") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the last time I asked about this is lost in community slack now, but we've been using passing the option multiple times as the preferred UX over comma separated or other list parsing.
--config-option
is one example of this: chef/chef#5045
We switched knife ec2 to this behavior for security group IDs: chef/knife-ec2#439
Also knife --chef-tag
and --aws-tag
: chef/knife-ec2#520
It's easy enough to split the directory on a comma Signed-off-by: Tim Smith <[email protected]>
Signed-off-by: Tim Smith <[email protected]>
Make sure it's an array before we .each it and add a spec that passes it as a string Signed-off-by: Tim Smith <[email protected]>
I wasn't aware that this was our standard way of doing this. Works for me. Signed-off-by: Tim Smith <[email protected]>
@btm @jaymzh @lamont-granquist This updated method work for you? |
This is required for passing an option more than once. Signed-off-by: Tim Smith <[email protected]>
Yeah that works! Thanks. |
This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
This allows you to run ohai -d /foo/bar,/bar/foo