Fixes #20804 - Support for deprecating commands #251
Conversation
|
@tstrachota hey, i have tried doing this, it is not working as expected, can you give some input to it ? |
|
@rahulbajaj0509 what exactly does not work for you? I gave it a try and it seems to work: HammerCLI::MainCommand.lazy_subcommand('architecture', _("Manipulate architectures."),
'HammerCLIForeman::Architecture', 'hammer_cli_foreman/architecture',
true
)...and the architecture is gone from We need to extend also |
rabajaj0509
force-pushed
the
hiddenFunctionality
branch
from
453f694
to
e48341a
Compare
|
@tstrachota actually the lazy_subcommand that i was passing was something like : So, that is the reason it was not working for me. |
rabajaj0509
force-pushed
the
hiddenFunctionality
branch
from
e48341a
to
74a68ac
Compare
rabajaj0509
force-pushed
the
hiddenFunctionality
branch
from
2c9941b
to
c1068ba
Compare
rabajaj0509
changed the title
lib/hammer_cli/subcommand.rb
Outdated
| logger.info "subcommand #{name} (#{subcommand_class_name}) was created." | ||
| end | ||
|
|
||
| def define_subcommand(subcommand_class, definition, &block) | ||
| existing = find_subcommand(name) |
There was a problem hiding this comment.
@rahulbajaj0509 ,
Does name accessible inside this method? Everytime you will get nil value for existing.
There was a problem hiding this comment.
Thanks @kgaikwad :) i had not added the name parameter, therefore my test was failing.
rabajaj0509
force-pushed
the
hiddenFunctionality
branch
from
c1068ba
to
fdd4c86
Compare
|
@tstrachota i am not sure why |
|
[test] |
|
@rahulbajaj0509 The code looks good now. I missed the failing test and even merged it but then I realized the test failure is related and had to revert it. There's problem with Clamp on ruby 2.0 where we have it pinned to < 1.1.0. Clamp dropped 2.0 support since then. This is the commit that we're missing: I haven't found any straightforward and clean way of adding the compatibility yet. @rahulbajaj0509 @mbacovsky any ideas? |
|
[test] |
|
👍 thanks @rahulbajaj0509 |
| @@ -59,23 +97,10 @@ def subcommand(name, description, subcommand_class = self, &block) | |||
| :new_class => subcommand_class | |||
| } | |||
| end | |||
| super | |||
| subcommand_class = Class.new(subcommand_class, &block) if block | |||
| declare_subcommand_parameters unless has_subcommands? | |||
There was a problem hiding this comment.
This line breaks systest: https://ci.theforeman.org/job/systest_foreman/13810/console
not ok 16 check smart proxy is registered and Hammer runs
# (in test file /usr/bin/fb-install-foreman.bats, line 167)
# `count=$(hammer $(tHammerCredentials) --csv proxy list | wc -l)' failed
# /opt/theforeman/tfm/root/usr/share/gems/gems/hammer_cli-0.12.pre.develop/lib/hammer_cli/subcommand.rb:101:in `define_subcommand': undefined local variable or method `declare_subcommand_parameters' for HammerCLI::DefaultsCommand:Class (NameError)
# from /opt/theforeman/tfm/root/usr/share/gems/gems/hammer_cli-0.12.pre.develop/lib/hammer_cli/subcommand.rb:77:in `subcommand'
# from /opt/theforeman/tfm/root/usr/share/gems/gems/hammer_cli-0.12.pre.develop/lib/hammer_cli/abstract.rb:212:in `block in autoload_subcommands'
# from /opt/theforeman/tfm/root/usr/share/gems/gems/hammer_cli-0.12.pre.develop/lib/hammer_cli/abstract.rb:211:in `each'
# from /opt/theforeman/tfm/root/usr/share/gems/gems/hammer_cli-0.12.pre.develop/lib/hammer_cli/abstract.rb:211:in `autoload_subcommands'
# from /opt/theforeman/tfm/root/usr/share/gems/gems/hammer_cli-0.12.pre.develop/lib/hammer_cli/defaults_commands.rb:158:in `<class:DefaultsCommand>'
# from /opt/theforeman/tfm/root/usr/share/gems/gems/hammer_cli-0.12.pre.develop/lib/hammer_cli/defaults_commands.rb:22:in `<module:HammerCLI>'
# from /opt/theforeman/tfm/root/usr/share/gems/gems/hammer_cli-0.12.pre.develop/lib/hammer_cli/defaults_commands.rb:3:in `<top (required)>'
# from /opt/rh/rh-ruby22/root/usr/share/rubygems/rubygems/core_ext/kernel_require.rb:54:in `require'
# from /opt/rh/rh-ruby22/root/usr/share/rubygems/rubygems/core_ext/kernel_require.rb:54:in `require'
# from /opt/theforeman/tfm/root/usr/share/gems/gems/hammer_cli-0.12.pre.develop/lib/hammer_cli/defaults.rb:1:in `<top (required)>'
# from /opt/rh/rh-ruby22/root/usr/share/rubygems/rubygems/core_ext/kernel_require.rb:54:in `require'
# from /opt/rh/rh-ruby22/root/usr/share/rubygems/rubygems/core_ext/kernel_require.rb:54:in `require'
# from /opt/theforeman/tfm/root/usr/share/gems/gems/hammer_cli-0.12.pre.develop/lib/hammer_cli/context.rb:1:in `<top (required)>'
# from /opt/rh/rh-ruby22/root/usr/share/rubygems/rubygems/core_ext/kernel_require.rb:54:in `require'
# from /opt/rh/rh-ruby22/root/usr/share/rubygems/rubygems/core_ext/kernel_require.rb:54:in `require'
# from /opt/theforeman/tfm/root/usr/share/gems/gems/hammer_cli-0.12.pre.develop/lib/hammer_cli.rb:17:in `<top (required)>'
# from /opt/rh/rh-ruby22/root/usr/share/rubygems/rubygems/core_ext/kernel_require.rb:54:in `require'
# from /opt/rh/rh-ruby22/root/usr/share/rubygems/rubygems/core_ext/kernel_require.rb:54:in `require'
# from /opt/theforeman/tfm/root/usr/share/gems/gems/hammer_cli-0.12.pre.develop/bin/hammer:117:in `<top (required)>'
# from /usr/bin/hammer:23:in `load'
# from /usr/bin/hammer:23:in `<main>'
There was a problem hiding this comment.
@ekohl this change require new version of clamp. The packaging PRs are theforeman/foreman-packaging#1872 (RPM) and theforeman/foreman-packaging#1874 (DEB).
No description provided.