Accelerated up autoconfig #240
Conversation
mchf
force-pushed
the
bnc886434-speed-up-autoconfig
branch
from
979385c
to
934d11d
Compare
| # @return true if configuration was reloaded | ||
| def reload_config(devs) | ||
| return false if devs.nil? || devs.empty? | ||
| SCR.Execute(BASH_PATH, "wicked ifreload '#{devs.join(" ")}'") == 0 |
There was a problem hiding this comment.
Just to be sure: Does wicked really take the device list in a single parameter with space separator? I'd expect multiple parameters or comma as a separator...
There was a problem hiding this comment.
I'd expect multiple parameters
this one
mchf
force-pushed
the
bnc886434-speed-up-autoconfig
branch
from
934d11d
to
70f3c7f
Compare
| # Reloads configuration for each device named in devs | ||
| # | ||
| # @devs list of device names | ||
| # @return true if configuration was reloaded |
There was a problem hiding this comment.
But you are not using this return value! Overspecifying only adds work without adding value.
|
Please pardon me for dissecting a simple PR. If you are in a hurry, just call so. I believe that thinking about the interface of the methods early will make our work easier when we have to revisit them. So, don't bother specifying the return value here, but be even more specific about the input. |
|
Well the intention was to have a quick fix, but i want to have at least new parts of yast2 lan in good shape, so I've updated the code a bit more. |
mchf
force-pushed
the
bnc886434-speed-up-autoconfig
branch
from
70f3c7f
to
fa00ac5
Compare
| @@ -194,7 +198,11 @@ def target_servers | |||
| # Check if given device can reach some of reference servers | |||
| def set_default_route_flag_if_wan_dev?(devname) | |||
| set_default_route_flag(devname, "yes") | |||
| activate_changes([devname]) | |||
|
|
|||
| if !activate_changes([devname]) | |||
There was a problem hiding this comment.
OK. One last thing, now that we do use the return value of activate_changes, that method should be documented to return a boolean status.
mchf
force-pushed
the
bnc886434-speed-up-autoconfig
branch
from
fa00ac5
to
7921cfd
Compare
|
LGTM, thank you. There should be no problems resolving the conflict with master. |
Configuration used to be reloaded one-by-one for multiple devices. Reload is issued just once since now.
mchf
force-pushed
the
bnc886434-speed-up-autoconfig
branch
from
7921cfd
to
0dc09b3
Compare
|
Rebased. Thanks for reviews |
bnc#886434