-
Notifications
You must be signed in to change notification settings - Fork 34
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
set more connection config attributes #898
Conversation
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.
In general it looks good. We could improve connection configs API to simpify this code.
@@ -58,12 +60,17 @@ def self.for(type) | |||
# | |||
# Load with reasonable defaults | |||
# @param type [Y2Network::InterfaceType] type of device | |||
def initialize(type:) | |||
# @param config [Y2Network::ConnectionConfig::Base] existing configuration of device or nil for |
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.
np: Just add the , nil
there.
|
||
# connection config | ||
# keep only default as aliases does not handle default ip config | ||
@connection_config.ip_configs.delete_if { |c| c.id != "" } |
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.
We could add ConnectionConfig::Base#main_ip_config
and ConnectionConfig::Base#aliases
(or something like this) so you do not have to worry about this.
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.
yep, when added, feel free to change this code ;)
# in such case remove default config | ||
@connection_config.ip_configs.delete_if { |c| c.id == "" } | ||
else | ||
ip_config_default.address.address = value |
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.
Same than above.
No description provided.