[pve-devel] [RFC installer] add button for renewing dhcp lease

[Thomas Lamprecht]

On 1/30/19 3:39 PM, Oguz Bektas wrote:
> per request on #2066, this is more of a proof-of-concept and not
> thoroughly tested.
> (i'm aware the button doesn't look very nice at the current position,
> planning to change it according to comments)

I've to agree that it really does not look very nice :D

> 
> Signed-off-by: Oguz Bektas <o.bektas at proxmox.com>
> ---
>  proxinstall | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/proxinstall b/proxinstall
> index f2414db..0422caa 100755
> --- a/proxinstall
> +++ b/proxinstall
> @@ -2013,6 +2013,13 @@ sub create_ipconf_view {
>      $device_cb->set_active(0);
>      $device_cb->set_visible(1);
>  

why putting this chunk of code in the mid of the $device_cb handling stuff?
I'd rather have it above the label create + pack hunk. Not that positioning
is so criitical, just do not place it in the middle of something.

> +    my $dhcp_button = Gtk3::Button->new("Renew DHCP");

I'd use something like:
Gtk3::Button->new_from_icon_name('view-refresh', 1);

plus a Tooltip for explain it a bit better for the confused ones:
$dhcp_button->set_tooltip_text("Retrigger DHCP request on interface");

(above text may not be ideal)

and place it besides the $device_cb

> +    $dhcp_button->signal_connect(clicked => sub {
> +	run_command("dhclient -r");
> +	run_command("dhclient -v");

so why did you chose this exact command set? Even if this is a POC for you,
description for why you do things like you do them is wanted.

why don't you pass only the currently selected NIC to dhclient? Wouldn't that
make sense at this stage?


> +	create_ipconf_view();

maybe we should only do this if there where actually changes?

> +    });
> +
>      my $get_device_desc = sub {
>  	my $iface = shift;
>  	return "$iface->{name} - $iface->{mac} ($iface->{driver})";
> @@ -2071,6 +2078,8 @@ sub create_ipconf_view {
>  
>      $vbox2->pack_start($maskbox, 0, 0, 2);
>  
> +    $vbox2->pack_end($dhcp_button, 0, 0, 2);
> +
>      $gateway = $config->{gateway} // $ipconf->{gateway} || '192.168.100.1';
>  
>      my $gwbox;
>