This repository has been archived by the owner on Aug 29, 2018. It is now read-only.
Fixes #21220 - make k-c-h command name easily changed #552
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Issues: #21220 |
johnpmitsch
force-pushed
the
kch_backport_name
branch
from
ab65102
to
5cb82b9
Compare
ehelms
reviewed
Oct 5, 2017
katello/katello-change-hostname
Outdated
| proxy: "Foreman Proxy", | ||
| plural_proxy: "Foreman Proxies", | ||
| proxy_hyphenated: "foreman-proxy-content", | ||
| plugin: "katello" |
There was a problem hiding this comment.
I think I might prefer this being command or command_name rather than plugin. Plugin tends to evoke a lot of other thoughts and this isn't really a plugin system.
|
@ehelms updated to |
johnpmitsch
force-pushed
the
kch_backport_name
branch
2 times, most recently
from
cafb928
to
a011f3e
Compare
ehelms
reviewed
Oct 6, 2017
katello/hostname-change.rb
Outdated
| @options[:program] = program || @default_program | ||
| @options[:scenario] = scenario || @last_scenario | ||
| @options[:program] = init_options[:program] || @default_program | ||
| @options[:scenario] = init_options[:scenario] || @last_scenario |
There was a problem hiding this comment.
Hashes have a builtin function for this .fetch(:scenario, @last_scenario)
ehelms
reviewed
Oct 6, 2017
katello/hostname-change.rb
Outdated
| @plural_proxy = init_options[:plural_proxy] | ||
| @proxy_hyphenated = init_options[:proxy_hyphenated] | ||
| @accepted_scenarios = init_options[:accepted_scenarios] | ||
| @command_prefix = init_options[:command_prefix] |
There was a problem hiding this comment.
If these are all required, you can use fetch to enforce that.
johnpmitsch
force-pushed
the
kch_backport_name
branch
from
a011f3e
to
2b74dc3
Compare
|
@ehelms thanks, updated |
ehelms
reviewed
Oct 6, 2017
katello/hostname-change.rb
Outdated
| @options[:program] = program || @default_program | ||
| @options[:scenario] = scenario || @last_scenario | ||
| @options[:program] = init_options[:program].fetch(:program, @default_program) | ||
| @options[:scenario] = init_options[:scenario].fetch(:scenario, @last_scenario) |
There was a problem hiding this comment.
I think you mean to leave off the [:...] for these two options?
johnpmitsch
force-pushed
the
kch_backport_name
branch
from
2b74dc3
to
cbc0014
Compare
ehelms
reviewed
Oct 6, 2017
katello/hostname-change.rb
Outdated
| @command_prefix = init_options.fetch(:command_prefix) | ||
| # accepted_scenarios is not a required parameter, so we allow to return nil. | ||
| # See method in helper file | ||
| @accepted_scenarios = init_options[:accepted_scenarios] |
There was a problem hiding this comment.
You can make this explicit without the need for comment by using init_options.fetch(:accepted_scenarios, nil) which will set the default to nil.
johnpmitsch
force-pushed
the
kch_backport_name
branch
from
cbc0014
to
379c032
Compare
ehelms
approved these changes
Oct 6, 2017
We need this to backport these changes.
johnpmitsch
force-pushed
the
kch_backport_name
branch
from
379c032
to
b9353c3
Compare
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We need this to backport these changes.