Re: [pve-devel] [PATCH manager 2/5 v2] GUI: Utils: Helpers for backup type and icon

[Thomas Lamprecht <t.lamprecht@proxmox.com>]

On 18/03/2022 14:52, Matthias Heiserer wrote:
> Signed-off-by: Matthias Heiserer <m.heiserer@proxmox.com>
> ---
> changes from v1:
> add "backup" to name
> return empty string instead of throwing


patch order is wrong though, you cannot use a helper in patch 1/x and only introduce
it in a later patch. This break bisect and causal order, both things I'm fond of :)

order can be fixed on apply for this one, at least if there's nothing else that comes up,
but please avoid that pattern for future series (or if this one gets a v3)

>  www/manager6/Utils.js | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/www/manager6/Utils.js b/www/manager6/Utils.js
> index aafe359a..337ccfae 100644
> --- a/www/manager6/Utils.js
> +++ b/www/manager6/Utils.js
> @@ -1803,6 +1803,26 @@ Ext.define('PVE.Utils', {
>  
>  	return undefined;
>      },
> +
> +    get_backup_type_icon_cls: function(volid, format) {
> +	var cls = '';

nit:
we use `let`  for new code, but actually I'd just return directly, like you do in the
other helper, the intermediate variable has no real benefit in this small, straight
forward function.

> +	if (PVE.Utils.volume_is_qemu_backup(volid, format)) {
> +	    cls = 'fa-desktop';
> +	} else if (PVE.Utils.volume_is_lxc_backup(volid, format)) {
> +	    cls = 'fa-cube';
> +	}
> +	return cls;
> +    },
> +
> +    get_backup_type: function(volid, format) {
> +	if (PVE.Utils.volume_is_qemu_backup(volid, format)) {
> +	    return 'qemu';
> +	} else if (PVE.Utils.volume_is_lxc_backup(volid, format)) {
> +	    return 'lxc';
> +	} else {
> +	    return '';
> +	}
> +    },
>  },
>  
>      singleton: true,