-
Notifications
You must be signed in to change notification settings - Fork 681
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
semantics: rename CLI plugins registry -> commands #435
Conversation
@chris-rock not sure if in or out, what do you think? |
@@ -143,7 +143,7 @@ class Inspec::InspecCLI < Inspec::BaseCLI # rubocop:disable Metrics/ClassLength | |||
end | |||
|
|||
# load CLI plugins before the Inspec CLI has been started | |||
Inspec::Plugins::CLI.registry.each { |_subcommand, params| | |||
Inspec::Plugins::CLI.commands.each { |_subcommand, params| |
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.
maybe we should use subcommands
to be very explicit
+1 i like it, since this solves some confusion |
Thank you for your feedback, I like |
eaacc55
to
9cfc067
Compare
@@ -18,11 +18,11 @@ | |||
before do | |||
# TODO: remove this, once the plugin loading is not based on ruby-load time | |||
# initialization | |||
cli_reg.registry.clear | |||
cli_reg.subcommands.clear |
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.
btw: i don't think these lines go away with ruby-load time initialization (which we changed in #434). e.g. if the second test (which adds subcommand) runs before the first one, the command is still in the registry.
imho: this line must stay in because right now we are dealing with a global singleton registry.
Basically make sure everyone understands these are only subcommands. we might consider adding plugins for options or existing commands instead of new subcommands. this just ensures everyone knows what registry is for
semantics: rename CLI plugins registry -> commands
Basically make sure everyone understands these are only subcommands. we might consider adding plugins for options or existing commands instead of new subcommands. this just ensures everyone knows what registry is for