Skip to content
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

Fixes #10564 - Added support for deprecated options #169

Merged
merged 1 commit into from May 26, 2015

Conversation

mbacovsky
Copy link
Member

No description provided.

@domcleal
Copy link
Contributor

Note, missing # in the commit + title message.

@mbacovsky mbacovsky changed the title Fixed 10564 - Added support for deprecated options Fixes #10564 - Added support for deprecated options May 21, 2015
@mbacovsky
Copy link
Member Author

Fixed. Thanks @domcleal

@tstrachota
Copy link
Member

@mbacovsky could you please update the docs? https://github.com/theforeman/hammer-cli/blob/master/doc/creating_commands.md#option-builders It took me some time to find out how to use the feature.

Not sure if it's a problem but this approach does not work when you define the options separately:

option '--old', 'OLD', 'Old option'
option "--new", "NEW", "Replacing option", :deprecated => { "--old" => "Use --new instead" }

No warning is displayed when you do hammer command --old=1.

@mbacovsky
Copy link
Member Author

@tstrachota thanks for review. I updated the docs and added one more straightforward notation format.

In your example the :deprecated option has to be set on the option being depricated.

With the new notation is should look like:

option '--old', 'OLD', 'Old option', :deprecated => "Use --new instead"
option "--new", "NEW", "Replacing option"

or with the extended syntax:

option '--old', 'OLD', 'Old option', :deprecated => { '--old' => "Use --new instead" }
option "--new", "NEW", "Replacing option"

@tstrachota
Copy link
Member

👍 it's more intuitive now, thanks @mbacovsky

tstrachota pushed a commit that referenced this pull request May 26, 2015
Fixes #10564 - Added support for deprecated options
@tstrachota tstrachota merged commit 83f0877 into theforeman:master May 26, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants